Skip to content

Commit

Permalink
Fix RN redbox messages for syntax errors by including error messages …
Browse files Browse the repository at this point in the history
…in payload

With RN 0.52, when there was a redbox due to a syntax error in a source file (with regular, non-delta bundler), the redbox would say just "No message provided". The JSON that Metro sent to RN did not include a "message" field because `JSON.stringify(error)` does not include `message`.

Test Plan: Add a syntax error to one of the files in RNTester's JS and load the RNTester app (from RN master). See that the redbox now says there was a transform error with info about where the syntax error is.

Also tested adding a syntax error with HMR enabled and saw that the error `message` field was set in the payload as expected.

Also added a Jest test to Server-test.js.
  • Loading branch information
ide committed Jan 15, 2018
1 parent 23855d0 commit 6d5878d
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 3 deletions.
1 change: 1 addition & 0 deletions packages/metro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"mkdirp": "^0.5.1",
"request": "^2.79.0",
"rimraf": "^2.5.4",
"serialize-error": "^2.1.0",
"source-map": "^0.5.6",
"temp": "0.8.3",
"throat": "^4.1.0",
Expand Down
44 changes: 43 additions & 1 deletion packages/metro/src/HmrServer/__tests__/HmrServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('HmrServer', () => {
return deltaBundlerMock;
},
};
getDeltaTransformerMock.reporterMock = {
reporterMock = {
update: jest.fn(),
};

Expand Down Expand Up @@ -127,4 +127,46 @@ describe('HmrServer', () => {
done();
}, 30);
});

it('should return error messages when there is a transform error', async done => {
jest.useRealTimers();
const sendMessage = jest.fn();

await hmrServer.onClientConnect(
'/hot?bundleEntry=EntryPoint.js&platform=ios',
sendMessage,
);

deltaTransformerMock.getDelta.mockImplementation(async () => {
const transformError = new SyntaxError('test syntax error');
transformError.type = 'TransformError';
transformError.filename = 'EntryPoint.js';
transformError.lineNumber = 123;
throw transformError;
});

deltaTransformerMock.emit('change');

setTimeout(function() {
expect(JSON.parse(sendMessage.mock.calls[0][0])).toEqual({
type: 'update-start',
});
const sentErrorMessage = JSON.parse(sendMessage.mock.calls[1][0]);
expect(sentErrorMessage).toMatchObject({type: 'error'});
expect(sentErrorMessage.body).toMatchObject({
type: 'TransformError',
message: 'test syntax error',
errors: [
{
filename: 'EntryPoint.js',
lineNumber: 123,
},
],
});
expect(JSON.parse(sendMessage.mock.calls[2][0])).toEqual({
type: 'update-done',
});
done();
}, 30);
});
});
25 changes: 25 additions & 0 deletions packages/metro/src/Server/__tests__/Server-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,31 @@ describe('processRequest', () => {
expect(response.body).toEqual('{"delta": "bundle"}');
});
});

it('should include the error message for transform errors', () => {
Serializers.deltaBundle.mockImplementation(async () => {
const transformError = new SyntaxError('test syntax error');
transformError.type = 'TransformError';
transformError.filename = 'testFile.js';
transformError.lineNumber = 123;
throw transformError;
});

return makeRequest(requestHandler, 'index.delta?platform=ios').then(
function(response) {
expect(() => JSON.parse(response.body)).not.toThrow();
const body = JSON.parse(response.body);
expect(body).toMatchObject({
type: 'TransformError',
message: 'test syntax error',
});
expect(body.errors).toContainEqual({
filename: 'testFile.js',
lineNumber: 123,
});
},
);
});
});

describe('/onchange endpoint', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`getTransformCacheKeyFn Should return always the same key for the same params 1`] = `"19a529ca8ec00ec50dbbf2de5278262868c1a04f32be6252a9bc5b8df65171914db10f84"`;
exports[`getTransformCacheKeyFn Should return always the same key for the same params 1`] = `"19a529ca8ec00ec50dbbf2de5278262868c1a04f22073b63c802f6d42d41280131b93e42"`;
4 changes: 3 additions & 1 deletion packages/metro/src/lib/formatBundlingError.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

'use strict';

const serializeError = require('serialize-error');

const {
UnableToResolveError,
} = require('../node-haste/DependencyGraph/ModuleResolution');
Expand Down Expand Up @@ -67,7 +69,7 @@ function formatBundlingError(
},
];

return error;
return serializeError(error);
} else {
return {
type: 'InternalError',
Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4869,6 +4869,10 @@ [email protected]:
range-parser "~1.0.3"
statuses "~1.2.1"

serialize-error@^2.1.0:
version "2.1.0"
resolved "https://registry.yarnpkg.com/serialize-error/-/serialize-error-2.1.0.tgz#50b679d5635cdf84667bdc8e59af4e5b81d5f60a"

serve-favicon@~2.3.0:
version "2.3.2"
resolved "https://registry.yarnpkg.com/serve-favicon/-/serve-favicon-2.3.2.tgz#dd419e268de012ab72b319d337f2105013f9381f"
Expand Down

0 comments on commit 6d5878d

Please sign in to comment.