Skip to content

Commit

Permalink
Symbolicate stack traces off the main process
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@davidaurelio @facebook-github-bot
davidaurelio authored and facebook-github-bot committed Feb 27, 2017
1 parent 17c175a commit b6ca952
Show file tree
Hide file tree
Showing 11 changed files with 685 additions and 91 deletions.
2 changes: 2 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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": {
Expand Down
4 changes: 3 additions & 1 deletion packager/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"
}
}
85 changes: 33 additions & 52 deletions packager/src/Server/__tests__/Server-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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;
Expand Down Expand Up @@ -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',
}));
Expand Down Expand Up @@ -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}));
});
});

Expand Down
71 changes: 33 additions & 38 deletions packager/src/Server/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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,
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -281,6 +283,8 @@ class Server {
}
this._informChangeWatchers();
}, 50);

this._symbolicateInWorker = symbolicate.createWorker();
}

end(): mixed {
Expand Down Expand Up @@ -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 => {
Expand All @@ -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: {
Expand Down Expand Up @@ -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;
Loading

0 comments on commit b6ca952

Please sign in to comment.