diff --git a/README.md b/README.md index cb6c9848..5805a916 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/__tests__/runtime/route.test.ts b/__tests__/runtime/route.test.ts index a13d454e..9e4d3cdd 100644 --- a/__tests__/runtime/route.test.ts +++ b/__tests__/runtime/route.test.ts @@ -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; @@ -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, + }); }); }); diff --git a/__tests__/utils/stack-trace/clean-up.test.ts b/__tests__/utils/stack-trace/clean-up.test.ts new file mode 100644 index 00000000..5306890c --- /dev/null +++ b/__tests__/utils/stack-trace/clean-up.test.ts @@ -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'); + }); +}); diff --git a/__tests__/utils/stack-trace/helpers.test.ts b/__tests__/utils/stack-trace/helpers.test.ts new file mode 100644 index 00000000..0b6aee19 --- /dev/null +++ b/__tests__/utils/stack-trace/helpers.test.ts @@ -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] + `); + }); +}); diff --git a/examples/basic/functions/error.js b/examples/basic/functions/error.js new file mode 100644 index 00000000..20d07cd9 --- /dev/null +++ b/examples/basic/functions/error.js @@ -0,0 +1,7 @@ +/// + +exports.handler = function(context, event, callback) { + const err = new Error('Something went wrong'); + err.name = 'WebhookError'; + callback(err); +}; diff --git a/package.json b/package.json index 0a1f4908..f5b23cd8 100644 --- a/package.json +++ b/package.json @@ -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/", diff --git a/src/runtime/route.ts b/src/runtime/route.ts index 89673898..125c5d34 100644 --- a/src/runtime/route.ts +++ b/src/runtime/route.ts @@ -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'; @@ -67,6 +68,10 @@ 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, @@ -74,15 +79,21 @@ export function handleError( 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); } } diff --git a/src/utils/stack-trace/clean-up.ts b/src/utils/stack-trace/clean-up.ts new file mode 100644 index 00000000..d4b0210a --- /dev/null +++ b/src/utils/stack-trace/clean-up.ts @@ -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(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; +} diff --git a/src/utils/stack-trace/helpers.ts b/src/utils/stack-trace/helpers.ts new file mode 100644 index 00000000..b433720d --- /dev/null +++ b/src/utils/stack-trace/helpers.ts @@ -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]`; +}