From 8496d323b6335ad3e88effc62d3766f79b9ea4da Mon Sep 17 00:00:00 2001 From: dkundel Date: Sun, 4 Aug 2019 21:45:00 -0700 Subject: [PATCH] feat(runtime): support better error formatting This adds both browser detection for pretty error messages and adds support for string error messages fix #64, fix #65 --- __tests__/runtime/route.test.ts | 63 +++++++++++++++++++++++++++++++-- package-lock.json | 14 ++++++++ package.json | 2 ++ src/runtime/route.ts | 17 ++++++--- src/runtime/server.ts | 2 ++ 5 files changed, 92 insertions(+), 6 deletions(-) diff --git a/__tests__/runtime/route.test.ts b/__tests__/runtime/route.test.ts index e9c1437f..457e6434 100644 --- a/__tests__/runtime/route.test.ts +++ b/__tests__/runtime/route.test.ts @@ -5,6 +5,7 @@ import { Request as ExpressRequest, Response as ExpressResponse, } from 'express'; +import { Request as MockRequest } from 'jest-express/lib/request'; import { Response as MockResponse } from 'jest-express/lib/response'; import { twiml } from 'twilio'; import { StartCliConfig } from '../../src/config/start'; @@ -25,12 +26,70 @@ const mockResponse = (new MockResponse() as unknown) as ExpressResponse; mockResponse.type = jest.fn(() => mockResponse); describe('handleError function', () => { - test('calls correct response methods', () => { + test('returns string error', () => { + const mockRequest = (new MockRequest() as unknown) as ExpressRequest; + mockRequest['useragent'] = { + isDesktop: true, + isMobile: false, + } as ExpressUseragent.UserAgent; + + handleError('string error', mockRequest, mockResponse); + + expect(mockResponse.status).toHaveBeenCalledWith(500); + expect(mockResponse.send).toHaveBeenCalledWith('string error'); + }); + + test('handles objects as error argument', () => { + const mockRequest = (new MockRequest() as unknown) as ExpressRequest; + mockRequest['useragent'] = { + isDesktop: true, + isMobile: false, + } as ExpressUseragent.UserAgent; + + handleError({ errorMessage: 'oh no' }, mockRequest, mockResponse); + + expect(mockResponse.status).toHaveBeenCalledWith(500); + expect(mockResponse.send).toHaveBeenCalledWith({ errorMessage: 'oh no' }); + }); + + test('wraps error object for desktop requests', () => { + const mockRequest = (new MockRequest() as unknown) as ExpressRequest; + mockRequest['useragent'] = { + isDesktop: true, + isMobile: false, + } as ExpressUseragent.UserAgent; + const err = new Error('Failed to execute'); - handleError(err, (mockResponse as unknown) as ExpressResponse); + handleError(err, mockRequest, mockResponse); expect(mockResponse.status).toHaveBeenCalledWith(500); expect(mockResponse.send).toHaveBeenCalledWith(wrapErrorInHtml(err)); }); + + test('wraps error object for mobile requests', () => { + const mockRequest = (new MockRequest() as unknown) as ExpressRequest; + mockRequest['useragent'] = { + isDesktop: false, + isMobile: true, + } as ExpressUseragent.UserAgent; + + const err = new Error('Failed to execute'); + handleError(err, mockRequest, mockResponse); + expect(mockResponse.status).toHaveBeenCalledWith(500); + expect(mockResponse.send).toHaveBeenCalledWith(wrapErrorInHtml(err)); + }); + + test('returns string version of error for other requests', () => { + const mockRequest = (new MockRequest() as unknown) as ExpressRequest; + mockRequest['useragent'] = { + isDesktop: false, + isMobile: false, + } as ExpressUseragent.UserAgent; + + const err = new Error('Failed to execute'); + handleError(err, mockRequest, mockResponse); + expect(mockResponse.status).toHaveBeenCalledWith(500); + expect(mockResponse.send).toHaveBeenCalledWith(err.toString()); + }); }); describe('constructEvent function', () => { diff --git a/package-lock.json b/package-lock.json index c4dc1b1d..beab8cf4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9337,6 +9337,15 @@ "@types/range-parser": "*" } }, + "@types/express-useragent": { + "version": "0.2.21", + "resolved": "https://registry.npmjs.org/@types/express-useragent/-/express-useragent-0.2.21.tgz", + "integrity": "sha512-xVZ9GVsrYa5PFghnb9UGiWR0ZOrAWhxSwbTVhqI1tMGU0I1TnAvzCQi15P29GWhc8mR384IUEINlhrPy0mz1Pg==", + "dev": true, + "requires": { + "@types/express": "*" + } + }, "@types/got": { "version": "9.6.0", "resolved": "https://registry.npmjs.org/@types/got/-/got-9.6.0.tgz", @@ -12057,6 +12066,11 @@ } } }, + "express-useragent": { + "version": "1.0.13", + "resolved": "https://registry.npmjs.org/express-useragent/-/express-useragent-1.0.13.tgz", + "integrity": "sha512-3RGcGuXbyr/t62yyu3CRLFS9Fc30tWiVd6TQ1/C7mdQU4Yv7UPfSvtAkYfqoaH2FMkQvrXD7V/FtpFhy6MfUcg==" + }, "extend": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/extend/-/extend-3.0.2.tgz", diff --git a/package.json b/package.json index 80bdecd4..adcb8254 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ "debug": "^3.1.0", "dotenv": "^6.2.0", "express": "^4.16.3", + "express-useragent": "^1.0.13", "fast-redact": "^1.5.0", "got": "^9.6.0", "inquirer": "^6.5.0", @@ -82,6 +83,7 @@ "@types/common-tags": "^1.8.0", "@types/debug": "^4.1.4", "@types/dotenv": "^6.1.1", + "@types/express-useragent": "^0.2.21", "@types/got": "^9.6.0", "@types/jest": "^24.0.15", "@types/listr": "^0.14.0", diff --git a/src/runtime/route.ts b/src/runtime/route.ts index 11083b9f..acbfe702 100644 --- a/src/runtime/route.ts +++ b/src/runtime/route.ts @@ -65,13 +65,22 @@ export function constructGlobalScope(config: StartCliConfig): void { } export function handleError( - err: Error, + err: Error | string | object, + req: ExpressRequest, res: ExpressResponse, functionFilePath?: string ) { res.status(500); - res.type('text/html'); - res.send(wrapErrorInHtml(err, functionFilePath)); + if (!(err instanceof Error)) { + res.send(err); + } else { + if (req.useragent && (req.useragent.isDesktop || req.useragent.isMobile)) { + res.type('text/html'); + res.send(wrapErrorInHtml(err, functionFilePath)); + } else { + res.send(err.toString()); + } + } } export function isTwiml(obj: object): boolean { @@ -131,7 +140,7 @@ export function functionToRoute( ) { debug('Function execution %s finished', req.path); if (err) { - handleError(err, res, functionFilePath); + handleError(err, req, res, functionFilePath); return; } handleSuccess(responseObject, res); diff --git a/src/runtime/server.ts b/src/runtime/server.ts index dd674257..a0451d07 100644 --- a/src/runtime/server.ts +++ b/src/runtime/server.ts @@ -6,6 +6,7 @@ import express, { Request as ExpressRequest, Response as ExpressResponse, } from 'express'; +import userAgentMiddleware from 'express-useragent'; import nocache from 'nocache'; import { StartCliConfig } from '../config/start'; import { wrapErrorInHtml } from '../utils/error-html'; @@ -48,6 +49,7 @@ export async function createServer( debug('Starting server with config: %p', config); const app = express(); + app.use(userAgentMiddleware.express()); app.use(bodyParser.urlencoded({ extended: false })); app.use(bodyParser.json()); app.get('/favicon.ico', (req, res) => {