Skip to content

feat(server): hide internals from stack trace #105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,47 @@ app.all(handlerToExpressRoute(handler));
app.listen(3000, () => console.log('Server running on port 3000'));
```

## Error Handling in Dev Server

If your local Twilio Function throws an unhandled error or returns an `Error` instance via the `callback` method, we will return an HTTP status code of `500` and return the error object as JSON.

By default we will clean up the stack trace for you to remove internal code of the dev server and add it as `at [Twilio Dev Server internals]` into the stack trace.

An example would look like this:

```
Error: What?
at format (/Users/dkundel/dev/twilio-run/examples/basic/functions/hello.js:5:9)
at exports.handler (/Users/dkundel/dev/twilio-run/examples/basic/functions/hello.js:13:3)
at [Twilio Dev Server internals]
```

If you want to have the full un-modified stack trace instead, set the following environment variable, either in your Twilio Function or via `.env`:

```
TWILIO_SERVERLESS_FULL_ERRORS=true
```

This will result into a stack trace like this:

```
Error: What?
at format (/Users/dkundel/dev/twilio-run/examples/basic/functions/hello.js:5:9)
at exports.handler (/Users/dkundel/dev/twilio-run/examples/basic/functions/hello.js:13:3)
at twilioFunctionHandler (/Users/dkundel/dev/twilio-run/dist/runtime/route.js:125:13)
at app.all (/Users/dkundel/dev/twilio-run/dist/runtime/server.js:122:82)
at Layer.handle [as handle_request] (/Users/dkundel/dev/twilio-run/node_modules/express/lib/router/layer.js:95:5)
at next (/Users/dkundel/dev/twilio-run/node_modules/express/lib/router/route.js:137:13)
at next (/Users/dkundel/dev/twilio-run/node_modules/express/lib/router/route.js:131:14)
at next (/Users/dkundel/dev/twilio-run/node_modules/express/lib/router/route.js:131:14)
at next (/Users/dkundel/dev/twilio-run/node_modules/express/lib/router/route.js:131:14)
at next (/Users/dkundel/dev/twilio-run/node_modules/express/lib/router/route.js:131:14)
```

In general you'll want to use the cleaned-up stack trace since the internals might change throughout time.



## Contributing

This project welcomes contributions from the community. Please see the [`CONTRIBUTING.md`](CONTRIBUTING.md) file for more details.
Expand Down
8 changes: 7 additions & 1 deletion __tests__/runtime/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
} from '../../src/runtime/route';
import { EnvironmentVariablesWithAuth } from '../../src/types/generic';
import { wrapErrorInHtml } from '../../src/utils/error-html';
import { cleanUpStackTrace } from '../../src/utils/stack-trace/clean-up';

const { VoiceResponse, MessagingResponse, FaxResponse } = twiml;

Expand Down Expand Up @@ -87,9 +88,14 @@ describe('handleError function', () => {
} as ExpressUseragent.UserAgent;

const err = new Error('Failed to execute');
const cleanedupError = cleanUpStackTrace(err);
handleError(err, mockRequest, mockResponse);
expect(mockResponse.status).toHaveBeenCalledWith(500);
expect(mockResponse.send).toHaveBeenCalledWith(err.toString());
expect(mockResponse.send).toHaveBeenCalledWith({
message: 'Failed to execute',
name: 'Error',
stack: cleanedupError.stack,
});
});
});

Expand Down
37 changes: 37 additions & 0 deletions __tests__/utils/stack-trace/clean-up.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { cleanUpStackTrace } from '../../../src/utils/stack-trace/clean-up';

jest.mock('../../../src/utils/stack-trace/helpers', () => {
return {
...jest.requireActual('../../../src/utils/stack-trace/helpers'),
formatStackTraceWithInternals: jest
.fn()
.mockReturnValue('[mock stacktrace]'),
};
});

describe('cleanUpStackTrace', () => {
beforeEach(() => {
delete process.env.TWILIO_SERVERLESS_FULL_ERRORS;
});

afterAll(() => {
delete process.env.TWILIO_SERVERLESS_FULL_ERRORS;
});

test('overrides stack trace by default', () => {
const err = new Error('Hello');
cleanUpStackTrace(err);
expect(err.stack).toBe('[mock stacktrace]');
expect(err.name).toBe('Error');
expect(err.message).toBe('Hello');
});

test('leaves stack trace if env variable is set', () => {
process.env.TWILIO_SERVERLESS_FULL_ERRORS = 'true';
const err = new Error('Hello');
cleanUpStackTrace(err);
expect(err.stack).not.toBe('[mock stacktrace]');
expect(err.name).toBe('Error');
expect(err.message).toBe('Hello');
});
});
99 changes: 99 additions & 0 deletions __tests__/utils/stack-trace/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { stripIndent } from 'common-tags';
import path from 'path';
import {
formatStackTraceWithInternals,
MODULE_ROOT,
stringifyStackTrace,
} from '../../../src/utils/stack-trace/helpers';

function generateMockCallSite(
str = `someTest (randomFakeFile.js:10:10)`,
fileName = 'randomFakeFile.js'
) {
return {
getThis: jest.fn(),
getTypeName: jest.fn(),
getFunction: jest.fn(),
getFunctionName: jest.fn(),
getMethodName: jest.fn(),
getFileName: jest.fn().mockReturnValue(fileName),
getLineNumber: jest.fn(),
getColumnNumber: jest.fn(),
getEvalOrigin: jest.fn(),
isToplevel: jest.fn(),
isEval: jest.fn(),
isNative: jest.fn(),
isConstructor: jest.fn(),
isAsync: jest.fn(),
isPromiseAll: jest.fn(),
getPromiseIndex: jest.fn(),
toString: () => {
return str;
},
};
}

describe('MODULE_ROOT', () => {
test('module root points at root of source code', () => {
expect(MODULE_ROOT.endsWith('src')).toBe(true);
});
});

describe('stringifyStackTrace', () => {
test('formats error correctly', () => {
const err = new Error('Test error message');
err.name = 'FakeTestError';
const callsites = [generateMockCallSite()];
const stackString = stringifyStackTrace(err, callsites);
expect(stackString).toEqual(stripIndent`
FakeTestError: Test error message
at someTest (randomFakeFile.js:10:10)
`);
});

test('handles multiple callsites', () => {
const err = new Error('Test error message');
err.name = 'FakeTestError';
const callsites = [
generateMockCallSite(),
generateMockCallSite('anotherTest (randomFakeFile.js:20:0)'),
];
const stackString = stringifyStackTrace(err, callsites);
expect(stackString).toEqual(stripIndent`
FakeTestError: Test error message
at someTest (randomFakeFile.js:10:10)
at anotherTest (randomFakeFile.js:20:0)
`);
});
});

describe('formatStackTraceWithInternals', () => {
test('does not add "internals" footer if no internals', () => {
const err = new Error('Test error message');
err.name = 'FakeTestError';
const callsites = [generateMockCallSite()];
const stackString = formatStackTraceWithInternals(err, callsites);
expect(stackString).toEqual(stripIndent`
FakeTestError: Test error message
at someTest (randomFakeFile.js:10:10)
`);
});

test('adds "internals" footer if internals are present', () => {
const err = new Error('Test error message');
err.name = 'FakeTestError';
const callsites = [
generateMockCallSite(),
generateMockCallSite(
'randomInternalThatShouldBeRemoved (randomFakeFile.js:20:0)',
path.join(MODULE_ROOT, 'randomFakeFile.js')
),
];
const stackString = formatStackTraceWithInternals(err, callsites);
expect(stackString).toEqual(stripIndent`
FakeTestError: Test error message
at someTest (randomFakeFile.js:10:10)
at [Twilio Dev Server internals]
`);
});
});
7 changes: 7 additions & 0 deletions examples/basic/functions/error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// <reference path="../../../node_modules/@twilio-labs/serverless-runtime-types/index.d.ts"/>

exports.handler = function(context, event, callback) {
const err = new Error('Something went wrong');
err.name = 'WebhookError';
callback(err);
};
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
"standard-version": "^6.0.1",
"supertest": "^3.1.0",
"ts-jest": "^24.0.2",
"typescript": "^3.5.2"
"typescript": "^3.7.4"
},
"files": [
"bin/",
Expand Down
21 changes: 16 additions & 5 deletions src/runtime/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { checkForValidAccountSid } from '../checks/check-account-sid';
import { StartCliConfig } from '../config/start';
import { wrapErrorInHtml } from '../utils/error-html';
import { getDebugFunction } from '../utils/logger';
import { cleanUpStackTrace } from '../utils/stack-trace/clean-up';
import { Response } from './internal/response';
import * as Runtime from './internal/runtime';

Expand Down Expand Up @@ -67,22 +68,32 @@ export function constructGlobalScope(config: StartCliConfig): void {
}
}

function isError(obj: any): obj is Error {
return obj instanceof Error;
}

export function handleError(
err: Error | string | object,
req: ExpressRequest,
res: ExpressResponse,
functionFilePath?: string
) {
res.status(500);
if (!(err instanceof Error)) {
res.send(err);
} else {
if (isError(err)) {
const cleanedupError = cleanUpStackTrace(err);

if (req.useragent && (req.useragent.isDesktop || req.useragent.isMobile)) {
res.type('text/html');
res.send(wrapErrorInHtml(err, functionFilePath));
res.send(wrapErrorInHtml(cleanedupError, functionFilePath));
} else {
res.send(err.toString());
res.send({
message: cleanedupError.message,
name: cleanedupError.name,
stack: cleanedupError.stack,
});
}
} else {
res.send(err);
}
}

Expand Down
19 changes: 19 additions & 0 deletions src/utils/stack-trace/clean-up.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { formatStackTraceWithInternals } from './helpers';

/**
* Uses the V8 Stack Trace API to hide any twilio-run internals from stack traces
* https://v8.dev/docs/stack-trace-api
*
* @param err Instance that inherits from Error
*/
export function cleanUpStackTrace<T extends Error>(err: T): T {
if (typeof process.env.TWILIO_SERVERLESS_FULL_ERRORS !== 'undefined') {
return err;
}

const backupPrepareStackTrace = Error.prepareStackTrace;
Error.prepareStackTrace = formatStackTraceWithInternals;
err.stack = err.stack;
Error.prepareStackTrace = backupPrepareStackTrace;
return err;
}
62 changes: 62 additions & 0 deletions src/utils/stack-trace/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import path from 'path';

export const MODULE_ROOT = path.resolve(__dirname, '../../');

/**
* Turns a set of stack frames into a stack trace string equivalent to the one
* generated by V8
*
* https://v8.dev/docs/stack-trace-api
*
* @param err The error instance for this stack trace
* @param stack Callsite instances for each frame from V8
*/
export function stringifyStackTrace(err: Error, stack: NodeJS.CallSite[]) {
const callSiteStrings = stack
.map(callSite => {
return ` at ${callSite}`;
})
.join('\n');
const stackTrace = `${err.name}: ${err.message}\n${callSiteStrings}`;
return stackTrace;
}

/**
* Returns all stack frames until one is reached that comes from inside twilio-run
*
* https://v8.dev/docs/stack-trace-api
*
* @param stack Array of callsite instances from the V8 Stack Trace API
*/
export function filterCallSites(stack: NodeJS.CallSite[]): NodeJS.CallSite[] {
let indexOfFirstInternalCallSite = stack.findIndex(callSite =>
callSite.getFileName()?.includes(MODULE_ROOT)
);
indexOfFirstInternalCallSite =
indexOfFirstInternalCallSite === -1
? stack.length
: indexOfFirstInternalCallSite;
return stack.slice(0, indexOfFirstInternalCallSite);
}

/**
* Removes any stack traces that are internal to twilio-run and replaces it
* with one [Twilio Dev Server internals] statement.
*
* To be used with Error.prepareStackTrace from the V8 Stack Trace API
* https://v8.dev/docs/stack-trace-api
*
* @param err The error instance for this stack trace
* @param stack Callsite instances for each from from V8 Stack Trace API
*/
export function formatStackTraceWithInternals(
err: Error,
stack: NodeJS.CallSite[]
): string {
const filteredStack = filterCallSites(stack);
const stackTraceWithoutInternals = stringifyStackTrace(err, filteredStack);
if (filteredStack.length === stack.length) {
return stackTraceWithoutInternals;
}
return `${stackTraceWithoutInternals}\n at [Twilio Dev Server internals]`;
}