Symbolicate stack traces off the main process

Summary:
Moves stack trace symbolication to a worker process.

The worker process is spawned laziliy, and is treated as an exclusive resource (requests are queued).

This helps keeping the server process responsive when symbolicating.

Reviewed By: cpojer

Differential Revision: D4602722

fbshipit-source-id: 5da97e53afd9a1ab981c5ba4b02a7d1d869dee71

Author: davidaurelio

package.json

@@ -153,6 +153,7 @@
     "bser": "^1.0.2",
     "chalk": "^1.1.1",
     "commander": "^2.9.0",
+    "concat-stream": "^1.6.0",
     "connect": "^2.8.3",
     "core-js": "^2.2.2",
     "debug": "^2.2.0",
@@ -207,6 +208,7 @@
     "ws": "^1.1.0",
     "xcode": "^0.8.9",
     "xmldoc": "^0.4.0",
+    "xpipe": "^1.0.5",
     "yargs": "^6.4.0"
   },
   "devDependencies": {

packager/package.json

@@ -17,6 +17,7 @@
     "babel-register": "^6.18.0",
     "babylon": "^6.14.1",
     "chalk": "^1.1.1",
+    "concat-stream": "^1.6.0",
     "core-js": "^2.2.2",
     "debug": "^2.2.0",
     "denodeify": "^1.2.1",
@@ -38,6 +39,7 @@
     "throat": "^3.0.0",
     "uglify-js": "^2.6.2",
     "worker-farm": "^1.3.1",
-    "write-file-atomic": "^1.2.0"
+    "write-file-atomic": "^1.2.0",
+    "xpipe": "^1.0.5"
   }
 }

packager/src/Server/__tests__/Server-test.js

@@ -10,11 +10,14 @@
 
 jest.disableAutomock();
 
-jest.setMock('worker-farm', function() { return () => {}; })
-    .setMock('timers', { setImmediate: (fn) => setTimeout(fn, 0) })
-    .setMock('uglify-js')
-    .setMock('crypto')
-    .setMock('source-map', { SourceMapConsumer: function(fn) {}})
+jest.mock('worker-farm', () => () => () => {})
+    .mock('timers', () => ({ setImmediate: (fn) => setTimeout(fn, 0) }))
+    .mock('uglify-js')
+    .mock('crypto')
+    .mock(
+      '../symbolicate',
+      () => ({createWorker: jest.fn().mockReturnValue(jest.fn())}),
+    )
     .mock('../../Bundler')
     .mock('../../AssetServer')
     .mock('../../lib/declareOpts')
@@ -23,14 +26,14 @@ jest.setMock('worker-farm', function() { return () => {}; })
     .mock('../../lib/GlobalTransformCache');
 
 describe('processRequest', () => {
-  let SourceMapConsumer, Bundler, Server, AssetServer, Promise;
+  let Bundler, Server, AssetServer, Promise, symbolicate;
   beforeEach(() => {
     jest.resetModules();
-    SourceMapConsumer = require('source-map').SourceMapConsumer;
     Bundler = require('../../Bundler');
     Server = require('../');
     AssetServer = require('../../AssetServer');
     Promise = require('promise');
+    symbolicate = require('../symbolicate');
   });
 
   let server;
@@ -69,7 +72,7 @@ describe('processRequest', () => {
       Promise.resolve({
         getModules: () => [],
         getSource: () => 'this is the source',
-        getSourceMap: () => {},
+        getSourceMap: () => ({version: 3}),
         getSourceMapString: () => 'this is the source map',
         getEtag: () => 'this is an etag',
       }));
@@ -464,60 +467,38 @@ describe('processRequest', () => {
   });
 
   describe('/symbolicate endpoint', () => {
+    let symbolicationWorker;
+    beforeEach(() => {
+      symbolicationWorker = symbolicate.createWorker();
+      symbolicationWorker.mockReset();
+    });
+
     it('should symbolicate given stack trace', () => {
-      const body = JSON.stringify({stack: [{
+      const inputStack = [{
         file: 'http://foo.bundle?platform=ios',
         lineNumber: 2100,
         column: 44,
         customPropShouldBeLeftUnchanged: 'foo',
-      }]});
-
-      SourceMapConsumer.prototype.originalPositionFor = jest.fn((frame) => {
-        expect(frame.line).toEqual(2100);
-        expect(frame.column).toEqual(44);
-        return {
-          source: 'foo.js',
-          line: 21,
-          column: 4,
-        };
-      });
-
-      return makeRequest(
-        requestHandler,
-        '/symbolicate',
-        { rawBody: body }
-      ).then(response => {
-        expect(JSON.parse(response.body)).toEqual({
-          stack: [{
-            file: 'foo.js',
-            lineNumber: 21,
-            column: 4,
-            customPropShouldBeLeftUnchanged: 'foo',
-          }]
-        });
+      }];
+      const outputStack = [{
+        source: 'foo.js',
+        line: 21,
+        column: 4,
+      }];
+      const body = JSON.stringify({stack: inputStack});
+
+      expect.assertions(2);
+      symbolicationWorker.mockImplementation(stack => {
+        expect(stack).toEqual(inputStack);
+        return outputStack;
       });
-    });
-
-    it('ignores `/debuggerWorker.js` stack frames', () => {
-      const body = JSON.stringify({stack: [{
-        file: 'http://localhost:8081/debuggerWorker.js',
-        lineNumber: 123,
-        column: 456,
-      }]});
 
       return makeRequest(
         requestHandler,
         '/symbolicate',
-        { rawBody: body }
-      ).then(response => {
-        expect(JSON.parse(response.body)).toEqual({
-          stack: [{
-            file: 'http://localhost:8081/debuggerWorker.js',
-            lineNumber: 123,
-            column: 456,
-          }]
-        });
-      });
+        {rawBody: body},
+      ).then(response =>
+        expect(JSON.parse(response.body)).toEqual({stack: outputStack}));
     });
   });
 

packager/src/Server/index.js

@@ -15,12 +15,12 @@ const AssetServer = require('../AssetServer');
 const getPlatformExtension = require('../node-haste').getPlatformExtension;
 const Bundler = require('../Bundler');
 const MultipartResponse = require('./MultipartResponse');
-const SourceMapConsumer = require('source-map').SourceMapConsumer;
 
 const declareOpts = require('../lib/declareOpts');
 const defaults = require('../../defaults');
 const mime = require('mime-types');
 const path = require('path');
+const symbolicate = require('./symbolicate');
 const terminal = require('../lib/terminal');
 const url = require('url');
 
@@ -34,6 +34,7 @@ import type Bundle from '../Bundler/Bundle';
 import type {Reporter} from '../lib/reporting';
 import type {GetTransformOptions} from '../Bundler';
 import type GlobalTransformCache from '../lib/GlobalTransformCache';
+import type {SourceMap, Symbolicate} from './symbolicate';
 
 const {
   createActionStartEntry,
@@ -201,6 +202,7 @@ class Server {
   _debouncedFileChangeHandler: (filePath: string) => mixed;
   _hmrFileChangeListener: (type: string, filePath: string) => mixed;
   _reporter: Reporter;
+  _symbolicateInWorker: Symbolicate;
 
   constructor(options: Options) {
     this._opts = {
@@ -281,6 +283,8 @@ class Server {
       }
       this._informChangeWatchers();
     }, 50);
+
+    this._symbolicateInWorker = symbolicate.createWorker();
   }
 
   end(): mixed {
@@ -802,45 +806,27 @@ class Server {
 
       // In case of multiple bundles / HMR, some stack frames can have
       // different URLs from others
-      const urlIndexes = {};
-      const uniqueUrls = [];
+      const urls = new Set();
       stack.forEach(frame => {
         const sourceUrl = frame.file;
         // Skip `/debuggerWorker.js` which drives remote debugging because it
         // does not need to symbolication.
         // Skip anything except http(s), because there is no support for that yet
-        if (!urlIndexes.hasOwnProperty(sourceUrl) &&
+        if (!urls.has(sourceUrl) &&
             !sourceUrl.endsWith('/debuggerWorker.js') &&
             sourceUrl.startsWith('http')) {
-          urlIndexes[sourceUrl] = uniqueUrls.length;
-          uniqueUrls.push(sourceUrl);
+          urls.add(sourceUrl);
         }
       });
 
-      const sourceMaps = uniqueUrls.map(
-        sourceUrl => this._sourceMapForURL(sourceUrl)
-      );
-      return Promise.all(sourceMaps).then(consumers => {
-        return stack.map(frame => {
-          const sourceUrl = frame.file;
-          if (!urlIndexes.hasOwnProperty(sourceUrl)) {
-            return frame;
-          }
-          const idx = urlIndexes[sourceUrl];
-          const consumer = consumers[idx];
-          const original = consumer.originalPositionFor({
-            line: frame.lineNumber,
-            column: frame.column,
-          });
-          if (!original) {
-            return frame;
-          }
-          return Object.assign({}, frame, {
-            file: original.source,
-            lineNumber: original.line,
-            column: original.column,
-          });
-        });
+      const mapPromises =
+        Array.from(urls.values()).map(this._sourceMapForURL, this);
+
+      debug('Getting source maps for symbolication');
+      return Promise.all(mapPromises).then(maps => {
+        debug('Sending stacks and maps to symbolication worker');
+        const urlsToMaps = zip(urls.values(), maps);
+        return this._symbolicateInWorker(stack, urlsToMaps);
       });
     }).then(
       stack => {
@@ -858,16 +844,13 @@ class Server {
     );
   }
 
-  _sourceMapForURL(reqUrl: string): Promise<SourceMapConsumer> {
+  _sourceMapForURL(reqUrl: string): Promise<SourceMap> {
     const options = this._getOptionsFromUrl(reqUrl);
     const building = this._useCachedOrUpdateOrCreateBundle(options);
-    return building.then(p => {
-      const sourceMap = p.getSourceMap({
-        minify: options.minify,
-        dev: options.dev,
-      });
-      return new SourceMapConsumer(sourceMap);
-    });
+    return building.then(p => p.getSourceMap({
+      minify: options.minify,
+      dev: options.dev,
+    }));
   }
 
   _handleError(res: ServerResponse, bundleID: string, error: {
@@ -990,4 +973,16 @@ function contentsEqual(array: Array<mixed>, set: Set<mixed>): boolean {
   return array.length === set.size && array.every(set.has, set);
 }
 
+function* zip<X, Y>(xs: Iterable<X>, ys: Iterable<Y>): Iterable<[X, Y]> {
+  //$FlowIssue #9324959
+  const ysIter: Iterator<Y> = ys[Symbol.iterator]();
+  for (const x of xs) {
+    const y = ysIter.next();
+    if (y.done) {
+      return;
+    }
+    yield [x, y.value];
+  }
+}
+
 module.exports = Server;