Skip to content

Commit

Permalink
Handle source file / source map fetch errors correctly, add tests
Browse files Browse the repository at this point in the history
Summary:
While rewriting `Debugger.getScriptSource` messages to fetch code and source map over HTTP, we weren't checking the status code of the fetch calls. This diff fixes that and adds corresponding tests (as well as for the filesystem error case).

Changelog: [Internal]

Differential Revision: D51013054

fbshipit-source-id: 9c411c1a2624b1ab081d6aa45f99e1e66acdef4d
  • Loading branch information
motiz88 authored and facebook-github-bot committed Nov 6, 2023
1 parent 1ec329b commit b867a21
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,57 @@ describe.each(['HTTP', 'HTTPS'])(
}
});

test('handling of failure to fetch source map', async () => {
const {device, debugger_} = await createAndConnectTarget(
serverRef,
autoCleanup.signal,
{
app: 'bar-app',
id: 'page1',
title: 'bar-title',
vm: 'bar-vm',
},
);
try {
const scriptParsedMessage = await sendFromTargetToDebugger(
device,
debugger_,
'page1',
{
method: 'Debugger.scriptParsed',
params: {
sourceMapURL: `${serverRef.serverBaseUrl}/source-map-missing`,
},
},
);

// We don't rewrite the message in this case.
expect(scriptParsedMessage.params.sourceMapURL).toEqual(
`${serverRef.serverBaseUrl}/source-map-missing`,
);

// We send an error through to the debugger as a console message.
expect(debugger_.handle).toBeCalledWith(
expect.objectContaining({
method: 'Runtime.consoleAPICalled',
params: {
args: [
{
type: 'string',
value: expect.stringMatching('Failed to fetch source map'),
},
],
executionContextId: 0,
type: 'error',
},
}),
);
} finally {
device.close();
debugger_.close();
}
});

describe.each(['10.0.2.2', '10.0.3.2'])(
'%s aliasing to and from localhost',
sourceHost => {
Expand Down Expand Up @@ -326,6 +377,89 @@ describe.each(['HTTP', 'HTTPS'])(
debugger_.close();
}
});

test.each(['url', 'file'])(
'reports %s fetch error back to debugger',
async resourceType => {
const {device, debugger_} = await createAndConnectTarget(
serverRef,
autoCleanup.signal,
{
app: 'bar-app',
id: 'page1',
title: 'bar-title',
vm: 'bar-vm',
},
);
try {
await sendFromTargetToDebugger(device, debugger_, 'page1', {
method: 'Debugger.scriptParsed',
params: {
scriptId: 'script1',
url:
resourceType === 'url'
? `${serverRef.serverBaseUrl}/source-missing`
: '__fixtures__/mock-source-file.does-not-exist',
startLine: 0,
endLine: 0,
startColumn: 0,
endColumn: 0,
hash: createHash('sha256').update('foo').digest('hex'),
},
});
const response = await debugger_.sendAndGetResponse({
id: 1,
method: 'Debugger.getScriptSource',
params: {
scriptId: 'script1',
},
});

// We mark the request as failed.
expect(response).toEqual({
id: 1,
result: {
error: {
message: expect.stringMatching(
`Failed to fetch source ${resourceType}`,
),
},
},
});

// We also send an error through to the debugger as a console message.
expect(debugger_.handle).toBeCalledWith(
expect.objectContaining({
method: 'Runtime.consoleAPICalled',
params: {
args: [
{
type: 'string',
value: expect.stringMatching(
`Failed to fetch source ${resourceType}`,
),
},
],
executionContextId: 0,
type: 'error',
},
}),
);

// The device does not receive the getScriptSource request, since it
// is handled by the proxy.
expect(device.wrappedEventParsed).not.toBeCalledWith({
pageId: 'page1',
wrappedEvent: expect.objectContaining({
method: 'Debugger.getScriptSource',
}),
});
} finally {
device.close();
debugger_.close();
}
},
);
});
},
);
3 changes: 3 additions & 0 deletions packages/dev-middleware/src/inspector-proxy/Device.js
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,9 @@ export default class Device {

// $FlowFixMe[incompatible-call] Suppress arvr node-fetch flow error
const response = await fetch(url);
if (!response.ok) {
throw new Error('HTTP ' + response.status + ' ' + response.statusText);
}
const text = await response.text();
// Restrict the length to well below the 500MB limit for nodejs (leaving
// room some some later manipulation, e.g. base64 or wrapping in JSON)
Expand Down

0 comments on commit b867a21

Please sign in to comment.