Skip to content

Commit

Permalink
InspectorProxy: Don't pass an Error object to socket.close()
Browse files Browse the repository at this point in the history
Summary:
Currently our error handling for websocket connections in `InspectorProxy` passes whatever is thrown as an "error explanation" to `socket.close()`.

This argument expects `string | undefined` (NB `null` is also invalid)

In cases where we're catching and passing an `Error`, this causes the socket instance to emit its own `'error'`, which we don't have any listener for, causing a crash.

This diff uses `e?.toString() ?? 'Unknown error'` to:
 - Preserve the current behaviour for raw strings
 - Show `Error: ${e.message}` when `e instanceof Error`
 - Show `Unknown error` when `e == null`.

Changelog:
**Fix:** Prevent `InspectorProxy` connection errors crashing the process.

Reviewed By: motiz88

Differential Revision: D36479972

fbshipit-source-id: 622be8c5f6bc516dd3bf9115445c7fca9c841409
  • Loading branch information
robhogan authored and facebook-github-bot committed May 19, 2022
1 parent 05c83d7 commit fdc4ef1
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions packages/metro-inspector-proxy/src/InspectorProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class InspectorProxy {
});
} catch (e) {
console.error('error', e);
socket.close(INTERNAL_ERROR_CODE, e);
socket.close(INTERNAL_ERROR_CODE, e?.toString() ?? 'Unknown error');
}
});
return wss;
Expand Down Expand Up @@ -212,7 +212,7 @@ class InspectorProxy {
device.handleDebuggerConnection(socket, pageId);
} catch (e) {
console.error(e);
socket.close(INTERNAL_ERROR_CODE, e);
socket.close(INTERNAL_ERROR_CODE, e?.toString() ?? 'Unknown error');
}
});
return wss;
Expand Down

0 comments on commit fdc4ef1

Please sign in to comment.