Skip to content

Commit

Permalink
Report errors in the console
Browse files Browse the repository at this point in the history
Summary:
Error responses in CDP aren't generally displayed to the user. Currently, when metro-inspector-proxy fails to fetch a resource, we report success and embed the error message in the resource, allowing the user to see the error (e.g. if a source file failed to load, the debug client would still receive a success message, and display a "fake" source file containing the error).

This change makes metro-inspector-proxy report errors at the CDP layer (if applicable), and also send an error message to the console so the user can see it.

Reviewed By: newobj

Differential Revision: D43167737

fbshipit-source-id: e9ecf7f663e8f1aa8a6d48483bef0a0a7ca28ed8
  • Loading branch information
Matt Blagden authored and facebook-github-bot committed Feb 16, 2023
1 parent 6690b39 commit da8b41b
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 20 deletions.
68 changes: 48 additions & 20 deletions packages/metro-inspector-proxy/src/Device.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import type {
DebuggerRequest,
ErrorResponse,
GetScriptSourceRequest,
GetScriptSourceResponse,
MessageFromDevice,
Expand Down Expand Up @@ -385,11 +386,9 @@ class Device {
'data:application/json;charset=utf-8;base64,' +
new Buffer(sourceMap).toString('base64');
} catch (exception) {
payload.params.sourceMapURL =
'data:application/json;charset=utf-8;base64,' +
new Buffer(
`Failed to fetch source map: ${exception.message}`,
).toString('base64');
this._sendErrorToDebugger(
`Failed to fetch source map ${params.sourceMapURL}: ${exception.message}`,
);
}
}
}
Expand Down Expand Up @@ -497,16 +496,22 @@ class Device {
}
}

async _processDebuggerGetScriptSource(
_processDebuggerGetScriptSource(
req: GetScriptSourceRequest,
socket: typeof WS,
) {
let scriptSource = `Source for script with id '${req.params.scriptId}' was not found.`;

const sendResponse = () => {
const sendSuccessResponse = (scriptSource: string) => {
const result: GetScriptSourceResponse = {scriptSource};
socket.send(JSON.stringify({id: req.id, result}));
};
const sendErrorResponse = (error: string) => {
// Tell the client that the request failed
const result: ErrorResponse = {error: {message: error}};
socket.send(JSON.stringify({id: req.id, result}));

// Send to the console as well, so the user can see it
this._sendErrorToDebugger(error);
};

const pathToSource = this._scriptIdToSourcePathMapping.get(
req.params.scriptId,
Expand All @@ -515,25 +520,27 @@ class Device {
const httpURL = this._tryParseHTTPURL(pathToSource);
if (httpURL) {
this._fetchText(httpURL).then(
text => {
scriptSource = text;
sendResponse();
},
err => {
scriptSource = err.message;
sendResponse();
},
text => sendSuccessResponse(text),
err =>
sendErrorResponse(
`Failed to fetch source url ${pathToSource}: ${err.message}`,
),
);
} else {
let file;
try {
scriptSource = fs.readFileSync(
file = fs.readFileSync(
path.resolve(this._projectRoot, pathToSource),
'utf8',
);
} catch (err) {
scriptSource = err.message;
sendErrorResponse(
`Failed to fetch source file ${pathToSource}: ${err.message}`,
);
}
if (file) {
sendSuccessResponse(file);
}
sendResponse();
}
}
}
Expand Down Expand Up @@ -579,6 +586,27 @@ class Device {
}
return text;
}

_sendErrorToDebugger(message: string) {
const debuggerSocket = this._debuggerConnection?.socket;
if (debuggerSocket && debuggerSocket.readyState === WS.OPEN) {
debuggerSocket.send(
JSON.stringify({
method: 'Runtime.consoleAPICalled',
params: {
args: [
{
type: 'string',
value: message,
},
],
executionContextId: 0,
type: 'error',
},
}),
);
}
}
}

module.exports = Device;
6 changes: 6 additions & 0 deletions packages/metro-inspector-proxy/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ export type GetScriptSourceResponse = {
bytecode?: string,
};

export type ErrorResponse = {
error: {
message: string,
},
};

export type DebuggerRequest =
| SetBreakpointByUrlRequest
| GetScriptSourceRequest;

0 comments on commit da8b41b

Please sign in to comment.