Skip to content

Commit

Permalink
Fixed uncaughtException event listener not being removed (#1036)
Browse files Browse the repository at this point in the history
  • Loading branch information
appurva21 authored Sep 24, 2024
1 parent 1874841 commit f1eda36
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
unreleased:
fixed bugs:
- GH-1036 Fixed `uncaughtException` event listener not being removed
- GH-1034 Fixed an issue where sandbox crashes for large response body

5.1.2:
Expand Down
34 changes: 14 additions & 20 deletions lib/sandbox/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,32 +189,28 @@ module.exports = function (bridge, glob) {
timers.clearAll();
}

// remove listener of disconnection event
vault.dispose();
bridge.off(abortEventName);
bridge.off(responseEventName);
bridge.off(cookiesEventName);
bridge.off('uncaughtException', onError);

// fire extra execution error event
if (err) { onError(err); }

// @note delete response from the execution object to avoid dispatching
// the large response payload back due to performance reasons.
execution.response && (delete execution.response);
function complete () {
vault.dispose();
bridge.off(abortEventName);
bridge.off(responseEventName);
bridge.off(cookiesEventName);
bridge.off('uncaughtException', onError);

// @note delete response from the execution object to avoid dispatching
// the large response payload back due to performance reasons.
execution.response && (delete execution.response);
bridge.dispatch(executionEventName, err || null, execution);
}

if (dnd !== true) {
const dispatchExecutionResult = () => {
bridge.dispatch(executionEventName, err || null, execution);
};

// if execution is skipped, we dispatch execution completion
// event immediately to avoid any further processing else we
// dispatch it in next tick to allow any other pending events
// to be dispatched.
skippedExecution ?
dispatchExecutionResult() :
timers.wrapped.setImmediate(dispatchExecutionResult);
skippedExecution ? complete() : timers.wrapped.setImmediate(complete);
}
});

Expand Down Expand Up @@ -249,9 +245,7 @@ module.exports = function (bridge, glob) {
// and one of them throws an error, this handler will be triggered
// for all of them. This is a limitation of uvm as there is no way
// to isolate the uncaught exception handling for each execution.
bridge.on('uncaughtException', (err) => {
onError(err);
});
bridge.on('uncaughtException', onError);

if (!options.resolvedPackages) {
disabledAPIs.push('require');
Expand Down

0 comments on commit f1eda36

Please sign in to comment.