Skip to content

Commit edd24f3

Browse files
Jack-OverflowJack-Overflow
authored
fix: fixing issue with async error handling within the SERVERLESS_EXPRESS:PROXY (#708)
* fix: stringify response headers as ALB expects response headers to be type string This was identified through using express within AWS Lambda which returns content-length header as a number which causes ALB to throw a 502 bad gateway exception. This change fixes this issue. * fix: stringify response headers as ALB expects response headers to be type string This was identified through using express within AWS Lambda which returns content-length header as a number which causes ALB to throw a 502 bad gateway exception. This change fixes this issue. * fix: fixing issue with async error handling within the SERVERLESS_EXPRESS:PROXY This change handles non-async and async errors thrown within `forwardRequestToNodeServer` and adds an integration test (this was previously skipped).
1 parent ce7ca85 commit edd24f3

File tree

6 files changed

+99
-48
lines changed

6 files changed

+99
-48
lines changed

‎__tests__/integration-alb.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const {
66
} = require('../jest-helpers')
77

88
describe('alb:express integration tests', () => {
9-
test('reasponse headers are of type string', async () => {
9+
test('response headers are of type string', async () => {
1010
const app = express()
1111
const router = express.Router()
1212
app.use('/', router)
@@ -15,14 +15,14 @@ describe('alb:express integration tests', () => {
1515
res.send('123')
1616
})
1717
const event = makeEvent({
18-
eventSourceName: 'alb',
18+
eventSourceName: 'AWS_ALB',
1919
path: '/foo',
2020
httpMethod: 'GET',
2121
headers: {}
2222
})
2323
const response = await serverlessExpressInstance(event)
2424
const expectedResponse = makeResponse({
25-
eventSourceName: 'alb',
25+
eventSourceName: 'AWS_ALB',
2626
body: '123',
2727
headers: {
2828
'content-length': '3',

‎__tests__/integration.js‎

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const {
1010
makeResponse,
1111
EACH_MATRIX
1212
} = require('../jest-helpers')
13+
const { getEventSource } = require('../src/event-sources')
1314
const jestHelpersPath = path.join(__dirname, '..', 'jest-helpers')
1415

1516
let app, router, serverlessExpressInstance
@@ -115,7 +116,7 @@ describe.each(EACH_MATRIX)('%s:%s: integration tests', (eventSourceName, framewo
115116
res.json({ xHeaders })
116117
})
117118
const event = makeEvent({
118-
eventSourceName: 'apiGatewayV1',
119+
eventSourceName: 'AWS_API_GATEWAY_V1',
119120
path: '/foo',
120121
httpMethod: 'GET',
121122
multiValueHeaders: undefined,
@@ -126,7 +127,7 @@ describe.each(EACH_MATRIX)('%s:%s: integration tests', (eventSourceName, framewo
126127
})
127128
const response = await serverlessExpressInstance(event)
128129
const expectedResponse = makeResponse({
129-
eventSourceName: 'apiGatewayV1',
130+
eventSourceName: 'AWS_API_GATEWAY_V1',
130131
body: JSON.stringify({
131132
xHeaders: {
132133
'x-header-one': 'Value1',
@@ -265,31 +266,31 @@ describe.each(EACH_MATRIX)('%s:%s: integration tests', (eventSourceName, framewo
265266
const etagRegex = /^W\/.*$/
266267
const lastModifiedRegex = /^.* GMT$/
267268
switch (eventSourceName) {
268-
case 'alb':
269-
case 'apiGatewayV1':
269+
case 'AWS_ALB':
270+
case 'AWS_API_GATEWAY_V1':
270271
expect(response.multiValueHeaders.etag.length).toEqual(1)
271272
expect(response.multiValueHeaders.etag[0]).toMatch(etagRegex)
272273
expect(response.multiValueHeaders['last-modified'].length).toEqual(1)
273274
expect(response.multiValueHeaders['last-modified'][0]).toMatch(lastModifiedRegex)
274275
delete response.multiValueHeaders.etag
275276
delete response.multiValueHeaders['last-modified']
276277
break
277-
case 'azureHttpFunctionV4':
278-
case 'azureHttpFunctionV3':
278+
case 'AZURE_HTTP_FUNCTION_V4':
279+
case 'AZURE_HTTP_FUNCTION_V3':
279280
expectedResponse.body = Buffer.from(samLogoBase64, 'base64')
280281
expectedResponse.isBase64Encoded = false
281282
expect(response.headers.etag).toMatch(etagRegex)
282283
expect(response.headers['last-modified']).toMatch(lastModifiedRegex)
283284
delete response.headers.etag
284285
delete response.headers['last-modified']
285286
break
286-
case 'apiGatewayV2':
287+
case 'AWS_API_GATEWAY_V2':
287288
expect(response.headers.etag).toMatch(etagRegex)
288289
expect(response.headers['last-modified']).toMatch(lastModifiedRegex)
289290
delete response.headers.etag
290291
delete response.headers['last-modified']
291292
break
292-
case 'lambdaEdge':
293+
case 'AWS_LAMBDA_EDGE':
293294
expect(response.headers.etag.length).toEqual(1)
294295
expect(response.headers.etag[0].key).toMatch('etag')
295296
expect(response.headers.etag[0].value).toMatch(etagRegex)
@@ -420,12 +421,56 @@ describe.each(EACH_MATRIX)('%s:%s: integration tests', (eventSourceName, framewo
420421
expect(response).toEqual(expectedResponse)
421422
})
422423

423-
test.skip('respondToEventSourceWithError', async () => {
424-
const response = await serverlessExpressInstance(null)
425-
expect(response).toEqual({
426-
statusCode: 500,
427-
body: '',
428-
multiValueHeaders: {}
424+
describe('respondToEventSourceWithError', () => {
425+
let event
426+
let eventSource
427+
let getRequestSpy
428+
429+
beforeEach(() => {
430+
event = makeEvent({
431+
eventSourceName,
432+
path: '/users',
433+
httpMethod: 'GET'
434+
})
435+
eventSource = getEventSource({ eventSourceName })
436+
getRequestSpy = jest.spyOn(eventSource, 'getRequest').mockImplementation(() => {
437+
throw new URIError('URI malformed')
438+
})
439+
})
440+
441+
afterEach(() => {
442+
getRequestSpy.mockRestore()
443+
})
444+
445+
test('respondWithErrors: true', async () => {
446+
serverlessExpressInstance = serverlessExpress({
447+
app,
448+
eventSource,
449+
respondWithErrors: true
450+
})
451+
452+
const response = await serverlessExpressInstance(event)
453+
const statusCode = response.statusCode || response.status
454+
455+
expect(statusCode).toBe(500)
456+
expect(response.body).toContain('URIError')
457+
expect(response.body).toContain('URI malformed')
458+
expect(getRequestSpy).toHaveBeenCalledWith(expect.objectContaining({ event }))
459+
})
460+
461+
test('respondWithErrors: false', async () => {
462+
serverlessExpressInstance = serverlessExpress({
463+
app,
464+
eventSource,
465+
respondWithErrors: false
466+
})
467+
468+
const response = await serverlessExpressInstance(event)
469+
const statusCode = response.statusCode || response.status
470+
471+
expect(statusCode).toBe(500)
472+
expect(response.body).toEqual('')
473+
expect(getRequestSpy).toHaveBeenCalledWith(expect.objectContaining({ event }))
429474
})
430475
})
431476

@@ -471,8 +516,8 @@ describe.each(EACH_MATRIX)('%s:%s: integration tests', (eventSourceName, framewo
471516
jest.useRealTimers()
472517

473518
switch (eventSourceName) {
474-
case 'azureHttpFunctionV4':
475-
case 'azureHttpFunctionV3':
519+
case 'AZURE_HTTP_FUNCTION_V4':
520+
case 'AZURE_HTTP_FUNCTION_V3':
476521
expectedResponse.cookies = [
477522
{
478523
domain: 'mafoo.com',

‎jest-helpers/index.js‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ const { makeAzureHttpFunctionV3Event, makeAzureHttpFunctionV3Response } = requir
66
const { makeAzureHttpFunctionV4Event, makeAzureHttpFunctionV4Response } = require('./azure-http-function-v4-event')
77

88
const EVENT_SOURCE_NAMES = [
9-
'alb',
10-
'apiGatewayV1',
11-
'apiGatewayV2',
12-
'lambdaEdge',
13-
'azureHttpFunctionV3',
14-
'azureHttpFunctionV4'
9+
'AWS_ALB',
10+
'AWS_API_GATEWAY_V1',
11+
'AWS_API_GATEWAY_V2',
12+
'AWS_LAMBDA_EDGE',
13+
'AZURE_HTTP_FUNCTION_V3',
14+
'AZURE_HTTP_FUNCTION_V4'
1515
]
1616

1717
const FRAMEWORK_NAMES = [
@@ -49,17 +49,17 @@ class MockContext {
4949

5050
function makeEvent ({ eventSourceName, ...rest }) {
5151
switch (eventSourceName) {
52-
case 'alb':
52+
case 'AWS_ALB':
5353
return makeAlbEvent(rest)
54-
case 'apiGatewayV1':
54+
case 'AWS_API_GATEWAY_V1':
5555
return makeApiGatewayV1Event(rest)
56-
case 'apiGatewayV2':
56+
case 'AWS_API_GATEWAY_V2':
5757
return makeApiGatewayV2Event(rest)
58-
case 'lambdaEdge':
58+
case 'AWS_LAMBDA_EDGE':
5959
return makeLambdaEdgeEvent(rest)
60-
case 'azureHttpFunctionV3':
60+
case 'AZURE_HTTP_FUNCTION_V3':
6161
return makeAzureHttpFunctionV3Event(rest)
62-
case 'azureHttpFunctionV4':
62+
case 'AZURE_HTTP_FUNCTION_V4':
6363
return makeAzureHttpFunctionV4Event(rest)
6464
default:
6565
throw new Error(`Unknown eventSourceName ${eventSourceName}`)
@@ -68,17 +68,17 @@ function makeEvent ({ eventSourceName, ...rest }) {
6868

6969
function makeResponse ({ eventSourceName, ...rest }, { shouldConvertContentLengthToInt = false } = {}) {
7070
switch (eventSourceName) {
71-
case 'alb':
71+
case 'AWS_ALB':
7272
return makeAlbResponse(rest)
73-
case 'apiGatewayV1':
73+
case 'AWS_API_GATEWAY_V1':
7474
return makeApiGatewayV1Response(rest)
75-
case 'apiGatewayV2':
75+
case 'AWS_API_GATEWAY_V2':
7676
return makeApiGatewayV2Response(rest, { shouldConvertContentLengthToInt })
77-
case 'lambdaEdge':
77+
case 'AWS_LAMBDA_EDGE':
7878
return makeLambdaEdgeResponse(rest)
79-
case 'azureHttpFunctionV3':
79+
case 'AZURE_HTTP_FUNCTION_V3':
8080
return makeAzureHttpFunctionV3Response(rest, { shouldConvertContentLengthToInt })
81-
case 'azureHttpFunctionV4':
81+
case 'AZURE_HTTP_FUNCTION_V4':
8282
return makeAzureHttpFunctionV4Response(rest, { shouldConvertContentLengthToInt })
8383
default:
8484
throw new Error(`Unknown eventSourceName ${eventSourceName}`)

‎package-lock.json‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/configure.js‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,17 @@ function configure ({
6868
promise,
6969
resolutionMode
7070
})
71+
const handleError = (error) => {
72+
respondToEventSourceWithError({
73+
error,
74+
resolver,
75+
log,
76+
respondWithErrors,
77+
eventSourceName,
78+
eventSource,
79+
event
80+
})
81+
}
7182

7283
try {
7384
forwardRequestToNodeServer({
@@ -81,16 +92,9 @@ function configure ({
8192
eventSource,
8293
eventSourceRoutes,
8394
log
84-
})
95+
}).catch(handleError)
8596
} catch (error) {
86-
respondToEventSourceWithError({
87-
error,
88-
resolver,
89-
log,
90-
respondWithErrors,
91-
eventSourceName,
92-
eventSource
93-
})
97+
handleError(error)
9498
}
9599
})
96100
}

‎src/transport.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ function respondToEventSourceWithError ({
5555
log,
5656
respondWithErrors,
5757
eventSourceName,
58-
eventSource
58+
eventSource,
59+
event
5960
}) {
6061
log.error('SERVERLESS_EXPRESS:RESPOND_TO_EVENT_SOURCE_WITH_ERROR', error)
6162

@@ -73,6 +74,7 @@ function respondToEventSourceWithError ({
7374

7475
const body = respondWithErrors ? error.stack : ''
7576
const errorResponse = eventSource.getResponse({
77+
event,
7678
statusCode: 500,
7779
body,
7880
headers: {},

0 commit comments

Comments
 (0)