Skip to content

Commit 53cf974

Browse files
committed
feat: add respondWithErrors config
* Changed from `logger.level === 'debug'` * Now responds with stack trace instead of just message * Added tests
1 parent a69af79 commit 53cf974

File tree

6 files changed

+83
-22
lines changed

6 files changed

+83
-22
lines changed

README.md

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,18 @@ app.get('/', (req, res) => {
4747

4848
## 4.0.0 Goals
4949

50-
1. Improved API - Simpler for end user to use and configure; extensible without breaking backwards compatibility or hurting API
51-
2. Node.js 8+ only - can upgrade dependencies to latest (Jest); can use latest syntax in source and tests; can use server.listening; future-proof for Node.js 10
52-
3. Promise resolution mode by default? Requires benchmarking. Otherwise try callback with callbackWaitsForEventLoop=false (configurable by user); requires benchmarking. If context.succeed is still most performant, leave as default.
53-
4. Additional event sources - currently only supports API Gateway Proxy; should also support Lambda@Edge (https://github.com/awslabs/aws-serverless-express/issues/152) and ALB; have had a customer request for DynamoDB; should make it easy to provide your own IO mapping function.
54-
5. Multiple header values - can get rid of set-cookie hack
55-
6. Configure logging - NONE, ERROR, INFO, DEBUG; also include option to respond to 500s with the stack trace instead of empty string currently
56-
7. Improved documentation
57-
8. Option to strip base path for custom domains (https://github.com/awslabs/aws-serverless-express/issues/86)
50+
1. ✅ Improved API - Simpler for end user to use and configure; extensible without breaking backwards compatibility or hurting API
51+
2. ✅ Node.js 8+ only - can upgrade dependencies to latest (Jest); can use latest syntax in source and tests; can use server.listening; future-proof for Node.js 10
52+
3. 🗓 Promise resolution mode by default? Requires benchmarking. Otherwise try callback with callbackWaitsForEventLoop=false (configurable by user); requires benchmarking. If context.succeed is still most performant, leave as default.
53+
4. 🗓 Additional event sources - currently only supports API Gateway Proxy; should also support Lambda@Edge (https://github.com/awslabs/aws-serverless-express/issues/152) and ALB; have had a customer request for DynamoDB; should make it easy to provide your own IO mapping function.
54+
1. Added ALB; requires refactoring of common logic, additional tests, example refactor.
55+
2. Need to add Lambda@Edge
56+
3. Need to add example of doing custom resolver (DynamoDB)
57+
5. ✅ Multiple header values - can get rid of set-cookie hack
58+
6. ✅ Configure logging - default winston and allow customers to provide their own; include option to respond to 500s with the stack trace instead of empty string currently
59+
7. 🗓Improved documentation
60+
8. ✅ Option to strip base path for custom domains (https://github.com/awslabs/aws-serverless-express/issues/86).
61+
9. 🗓 Update example to include optional parameter for setting up custom domain
5862

5963
### Is AWS serverless right for my app?
6064

__tests__/unit.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,29 @@ describe('forwardConnectionErrorResponseToApiGateway', () => {
162162
multiValueHeaders: {}
163163
}))
164164
})
165+
test('responds with 502 status and stack trace', () => {
166+
return new Promise(
167+
(resolve) => {
168+
const context = new MockContext(resolve)
169+
const contextResolver = {
170+
succeed: (p) => context.succeed(p.response)
171+
}
172+
awsServerlessExpressTransport.forwardConnectionErrorResponseToApiGateway({
173+
error: new Error('There was a connection error...'),
174+
resolver: contextResolver,
175+
logger,
176+
respondWithErrors: true
177+
})
178+
}
179+
).then(successResponse => {
180+
expect(successResponse).toEqual({
181+
statusCode: 502,
182+
body: successResponse.body,
183+
multiValueHeaders: {}
184+
})
185+
expect(successResponse.body).toContain('Error: There was a connection error...\n at ')
186+
})
187+
})
165188
})
166189

167190
describe('forwardLibraryErrorResponseToApiGateway', () => {
@@ -180,6 +203,29 @@ describe('forwardLibraryErrorResponseToApiGateway', () => {
180203
multiValueHeaders: {}
181204
}))
182205
})
206+
test('responds with 500 status and stack trace', () => {
207+
return new Promise(
208+
(resolve) => {
209+
const context = new MockContext(resolve)
210+
const contextResolver = {
211+
succeed: (p) => context.succeed(p.response)
212+
}
213+
awsServerlessExpressTransport.forwardLibraryErrorResponseToApiGateway({
214+
error: new Error('There was an error...'),
215+
resolver: contextResolver,
216+
logger,
217+
respondWithErrors: true
218+
})
219+
}
220+
).then(successResponse => {
221+
expect(successResponse).toEqual({
222+
statusCode: 500,
223+
body: successResponse.body,
224+
multiValueHeaders: {}
225+
})
226+
expect(successResponse.body).toContain('Error: There was an error...\n at ')
227+
})
228+
})
183229
})
184230

185231
function getContextResolver (resolve) {

examples/basic-starter/lambda.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ const binaryMimeTypes = [
1010
]
1111
const ase = awsServerlessExpress.configure({
1212
app,
13-
binaryMimeTypes
13+
binaryMimeTypes,
14+
respondWithErrors: true
1415
// loggerConfig: {
1516
// level: 'debug'
1617
// }

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@
117117
"scripts": {
118118
"test": "jest",
119119
"test:watch": "jest --watch",
120-
"coverage": "jest --coverage",
120+
"test:coverage": "jest --coverage",
121121
"cz": "git-cz",
122122
"release": "semantic-release",
123123
"release-local": "node -r dotenv/config node_modules/semantic-release/bin/semantic-release --no-ci --dry-run",

src/index.js

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ function proxy ({
7474
resolutionMode = 'CONTEXT_SUCCEED',
7575
eventSource = getEventSourceBasedOnEvent({ event }),
7676
eventFns = getEventFnsBasedOnEventSource({ eventSource }),
77-
logger
77+
logger,
78+
respondWithErrors
7879
}) {
7980
logger.debug('Calling proxy', { event, context, resolutionMode, eventSource })
8081
setCurrentLambdaInvoke({ event, context })
@@ -99,7 +100,8 @@ function proxy ({
99100
resolver,
100101
eventSource,
101102
eventFns,
102-
logger
103+
logger,
104+
respondWithErrors
103105
})
104106
} else {
105107
logger.debug('Server isn\'t listening... Starting server. This is likely a cold-start. If you see this message on every request, you may be calling `awsServerlessExpress.createServer` on every call inside the handler function. If this is the case, consider moving it outside of the handler function for imrpoved performance.')
@@ -113,7 +115,8 @@ function proxy ({
113115
resolver,
114116
eventSource,
115117
eventFns,
116-
logger
118+
logger,
119+
respondWithErrors
117120
})
118121
})
119122
}
@@ -141,6 +144,7 @@ function configure ({
141144
resolutionMode: configureResolutionMode = 'CONTEXT_SUCCEED',
142145
eventSource: configureEventSource,
143146
eventFns: configureEventFns,
147+
respondWithErrors: configureRespondWithErrors = false,
144148
loggerConfig: configureLoggerConfig = {},
145149
logger: configureLogger = createLogger({
146150
...DEFAULT_LOGGER_CONFIG,
@@ -164,7 +168,8 @@ function configure ({
164168
callback,
165169
eventSource = configureEventSource,
166170
eventFns = configureEventFns,
167-
logger = configureLogger
171+
logger = configureLogger,
172+
respondWithErrors = configureRespondWithErrors
168173
} = {}) => (proxy({
169174
server,
170175
event,
@@ -173,7 +178,8 @@ function configure ({
173178
callback,
174179
eventSource,
175180
eventFns,
176-
logger
181+
logger,
182+
respondWithErrors
177183
})),
178184
handler: configureHandler = (event, context, callback) => configureProxy({
179185
event,

src/transport.js

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ function forwardResponse ({
4646
})
4747
}
4848

49-
function forwardConnectionErrorResponseToApiGateway ({ error, resolver, logger }) {
49+
function forwardConnectionErrorResponseToApiGateway ({ error, resolver, logger, respondWithErrors }) {
5050
logger.error('aws-serverless-express connection error: ', error)
51-
const body = logger.level === 'debug' ? error.message : ''
51+
const body = respondWithErrors ? error.stack : ''
5252
const errorResponse = {
5353
statusCode: 502, // "DNS resolution, TCP level errors, or actual HTTP parse errors" - https://nodejs.org/api/http.html#http_http_request_options_callback
5454
body,
@@ -58,9 +58,10 @@ function forwardConnectionErrorResponseToApiGateway ({ error, resolver, logger }
5858
resolver.succeed({ response: errorResponse })
5959
}
6060

61-
function forwardLibraryErrorResponseToApiGateway ({ error, resolver, logger }) {
61+
function forwardLibraryErrorResponseToApiGateway ({ error, resolver, logger, respondWithErrors }) {
6262
logger.error('aws-serverless-express error: ', error)
63-
const body = logger.level === 'debug' ? error.message : ''
63+
64+
const body = respondWithErrors ? error.stack : ''
6465
const errorResponse = {
6566
statusCode: 500,
6667
body,
@@ -77,7 +78,8 @@ function forwardRequestToNodeServer ({
7778
resolver,
7879
eventSource,
7980
eventFns = getEventFnsBasedOnEventSource({ eventSource }),
80-
logger
81+
logger,
82+
respondWithErrors
8183
}) {
8284
logger.debug('Forwarding request to application...')
8385
try {
@@ -104,14 +106,16 @@ function forwardRequestToNodeServer ({
104106
.on('error', (error) => forwardConnectionErrorResponseToApiGateway({
105107
error,
106108
resolver,
107-
logger
109+
logger,
110+
respondWithErrors
108111
}))
109112
.end()
110113
} catch (error) {
111114
forwardLibraryErrorResponseToApiGateway({
112115
error,
113116
resolver,
114-
logger
117+
logger,
118+
respondWithErrors
115119
})
116120
}
117121
}

0 commit comments

Comments
 (0)