From 6d5878d562a941e209b608ab1196e078db5086dd Mon Sep 17 00:00:00 2001 From: James Ide Date: Sun, 14 Jan 2018 13:08:47 -0800 Subject: [PATCH] Fix RN redbox messages for syntax errors by including error messages 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. --- packages/metro/package.json | 1 + .../src/HmrServer/__tests__/HmrServer-test.js | 44 ++++++++++++++++++- .../metro/src/Server/__tests__/Server-test.js | 25 +++++++++++ .../getTransformCacheKeyFn-test.js.snap | 2 +- packages/metro/src/lib/formatBundlingError.js | 4 +- yarn.lock | 4 ++ 6 files changed, 77 insertions(+), 3 deletions(-) diff --git a/packages/metro/package.json b/packages/metro/package.json index f63799227d..706abbaac0 100644 --- a/packages/metro/package.json +++ b/packages/metro/package.json @@ -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", diff --git a/packages/metro/src/HmrServer/__tests__/HmrServer-test.js b/packages/metro/src/HmrServer/__tests__/HmrServer-test.js index b7ea9365b9..cf6fa15e36 100644 --- a/packages/metro/src/HmrServer/__tests__/HmrServer-test.js +++ b/packages/metro/src/HmrServer/__tests__/HmrServer-test.js @@ -42,7 +42,7 @@ describe('HmrServer', () => { return deltaBundlerMock; }, }; - getDeltaTransformerMock.reporterMock = { + reporterMock = { update: jest.fn(), }; @@ -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); + }); }); diff --git a/packages/metro/src/Server/__tests__/Server-test.js b/packages/metro/src/Server/__tests__/Server-test.js index c9e02e1246..3510b5d539 100644 --- a/packages/metro/src/Server/__tests__/Server-test.js +++ b/packages/metro/src/Server/__tests__/Server-test.js @@ -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', () => { diff --git a/packages/metro/src/lib/__tests__/__snapshots__/getTransformCacheKeyFn-test.js.snap b/packages/metro/src/lib/__tests__/__snapshots__/getTransformCacheKeyFn-test.js.snap index 0b6f849c4a..e0a1115535 100644 --- a/packages/metro/src/lib/__tests__/__snapshots__/getTransformCacheKeyFn-test.js.snap +++ b/packages/metro/src/lib/__tests__/__snapshots__/getTransformCacheKeyFn-test.js.snap @@ -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"`; diff --git a/packages/metro/src/lib/formatBundlingError.js b/packages/metro/src/lib/formatBundlingError.js index 84ebba780b..3bbdf9afe3 100644 --- a/packages/metro/src/lib/formatBundlingError.js +++ b/packages/metro/src/lib/formatBundlingError.js @@ -12,6 +12,8 @@ 'use strict'; +const serializeError = require('serialize-error'); + const { UnableToResolveError, } = require('../node-haste/DependencyGraph/ModuleResolution'); @@ -67,7 +69,7 @@ function formatBundlingError( }, ]; - return error; + return serializeError(error); } else { return { type: 'InternalError', diff --git a/yarn.lock b/yarn.lock index 1a05b8bfce..b320d93798 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4869,6 +4869,10 @@ send@0.13.2: 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"