From e87a8205d8ca070df0fcef67dbe4f02389770b95 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Wed, 14 Jun 2017 05:00:57 -0700 Subject: [PATCH] Handle synchronous errors in `worker.transformCode` Summary: `worker.transformCode` is a callback-taking function, but did not properly guard against errors thrown in its body. Reviewed By: cpojer Differential Revision: D5245253 fbshipit-source-id: 3fd08b68dd8605f664b316652ebd1f9497b2dac9 --- .../worker/__tests__/worker-test.js | 18 +++ .../src/JSTransformer/worker/index.js | 142 +++++++++--------- 2 files changed, 92 insertions(+), 68 deletions(-) diff --git a/packages/metro-bundler/src/JSTransformer/worker/__tests__/worker-test.js b/packages/metro-bundler/src/JSTransformer/worker/__tests__/worker-test.js index 6fde4d89..da0aa494 100644 --- a/packages/metro-bundler/src/JSTransformer/worker/__tests__/worker-test.js +++ b/packages/metro-bundler/src/JSTransformer/worker/__tests__/worker-test.js @@ -213,6 +213,24 @@ describe('code transformation worker:', () => { }, ); }); + + it('calls back with every error thrown by `extractDependencies`', done => { + const error = new Error('arbitrary'); + extractDependencies.mockImplementation(() => { + throw error; + }); + transformCode( + transformer, + 'arbitrary.js', + 'local/arbitrary.js', + 'code', + {}, + (e, data) => { + expect(e).toBe(error); + done(); + }, + ); + }); }); describe('Minifications:', () => { diff --git a/packages/metro-bundler/src/JSTransformer/worker/index.js b/packages/metro-bundler/src/JSTransformer/worker/index.js index 7ef9da0e..678a423f 100644 --- a/packages/metro-bundler/src/JSTransformer/worker/index.js +++ b/packages/metro-bundler/src/JSTransformer/worker/index.js @@ -12,6 +12,7 @@ 'use strict'; +const asyncify = require('async/asyncify'); const constantFolding = require('./constant-folding'); const extractDependencies = require('./extract-dependencies'); const inline = require('./inline'); @@ -73,89 +74,94 @@ export type Data = { }; type Callback = (error: ?Error, data: ?T) => mixed; +type TransformCode = ( + Transformer<*>, + string, + LocalPath, + string, + Options, + Callback, +) => void; -function transformCode( - transformer: Transformer<*>, - filename: string, - localPath: LocalPath, - sourceCode: string, - options: Options, - callback: Callback, -) { - invariant( - !options.minify || options.transform.generateSourceMaps, - 'Minifying source code requires the `generateSourceMaps` option to be `true`', - ); +const transformCode: TransformCode = asyncify( + ( + transformer: Transformer<*>, + filename: string, + localPath: LocalPath, + sourceCode: string, + options: Options, + ): Data => { + invariant( + !options.minify || options.transform.generateSourceMaps, + 'Minifying source code requires the `generateSourceMaps` option to be `true`', + ); - const isJson = filename.endsWith('.json'); - if (isJson) { - sourceCode = 'module.exports=' + sourceCode; - } + const isJson = filename.endsWith('.json'); + if (isJson) { + sourceCode = 'module.exports=' + sourceCode; + } - const transformFileStartLogEntry = { - action_name: 'Transforming file', - action_phase: 'start', - file_name: filename, - log_entry_label: 'Transforming file', - start_timestamp: process.hrtime(), - }; + const transformFileStartLogEntry = { + action_name: 'Transforming file', + action_phase: 'start', + file_name: filename, + log_entry_label: 'Transforming file', + start_timestamp: process.hrtime(), + }; - let transformed; - try { - transformed = transformer.transform({ + const transformed = transformer.transform({ filename, localPath, options: options.transform, src: sourceCode, }); - } catch (error) { - callback(error); - return; - } - invariant( - transformed != null, - 'Missing transform results despite having no error.', - ); + invariant( + transformed != null, + 'Missing transform results despite having no error.', + ); - var code, map; - if (options.minify) { - ({code, map} = constantFolding( - filename, - inline(filename, transformed, options), - )); - invariant(code != null, 'Missing code from constant-folding transform.'); - } else { - ({code, map} = transformed); - } + var code, map; + if (options.minify) { + ({code, map} = constantFolding( + filename, + inline(filename, transformed, options), + )); + invariant(code != null, 'Missing code from constant-folding transform.'); + } else { + ({code, map} = transformed); + } - if (isJson) { - code = code.replace(/^\w+\.exports=/, ''); - } else { - // Remove shebang - code = code.replace(/^#!.*/, ''); - } + if (isJson) { + code = code.replace(/^\w+\.exports=/, ''); + } else { + // Remove shebang + code = code.replace(/^#!.*/, ''); + } - const depsResult = isJson - ? {dependencies: [], dependencyOffsets: []} - : extractDependencies(code); + const depsResult = isJson + ? {dependencies: [], dependencyOffsets: []} + : extractDependencies(code); - const timeDelta = process.hrtime(transformFileStartLogEntry.start_timestamp); - const duration_ms = Math.round((timeDelta[0] * 1e9 + timeDelta[1]) / 1e6); - const transformFileEndLogEntry = { - action_name: 'Transforming file', - action_phase: 'end', - file_name: filename, - duration_ms, - log_entry_label: 'Transforming file', - }; + const timeDelta = process.hrtime( + transformFileStartLogEntry.start_timestamp, + ); + const duration_ms = Math.round((timeDelta[0] * 1e9 + timeDelta[1]) / 1e6); + const transformFileEndLogEntry = { + action_name: 'Transforming file', + action_phase: 'end', + file_name: filename, + duration_ms, + log_entry_label: 'Transforming file', + }; - callback(null, { - result: {...depsResult, code, map}, - transformFileStartLogEntry, - transformFileEndLogEntry, - }); -} + return { + result: {...depsResult, code, map}, + transformFileStartLogEntry, + transformFileEndLogEntry, + }; + }, +); exports.transformAndExtractDependencies = ( transform: string,