Skip to content
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

Serverless.com - throws error for sync lambda #860

Closed
subzero10 opened this issue Aug 16, 2022 · 1 comment · Fixed by #887
Closed

Serverless.com - throws error for sync lambda #860

subzero10 opened this issue Aug 16, 2022 · 1 comment · Fixed by #887
Assignees
Labels
js @honeybadger-io/js

Comments

@subzero10
Copy link
Member

What are the steps to reproduce this issue?

  1. Create a serverless project
  2. Create a simple synchronous handler that returns 200 (no callback, no async/await)
  3. Wrap handler with Honeybadger.lambdaHandler
  4. Do sls deploy

What happens?

Honeybadger client throws error.

What were you expecting to happen?

Should not throw error.

Any logs, error output, etc?

TypeError: handler(...).then is not a function
    at [NODE_MODULES]/@honeybadger-io/js/dist/server/honeybadger.js:1153:26
    at async_hooks.js:304:14
    at AsyncResource.runInAsyncScope (async_hooks.js:189:9)
    at AsyncLocalStorage.run (async_hooks.js:302:35)
    at [NODE_MODULES]/@honeybadger-io/js/dist/server/honeybadger.js:1149:40
    at new Promise (<anonymous>)
    at wrappedLambdaHandler ([NODE_MODULES]/@honeybadger-io/js/dist/server/honeybadger.js:1148:16)
    at Runtime.handler ([PROJECT_ROOT]/serverless_sdk/index.js:24:10716)
    at Runtime.handleOnce (/var/runtime/Runtime.js:66:25)

Any other comments?

The issue goes away if I use async/await.

What versions are you using?

Operating System: AWS Lambda - nodejs v12
Package Version: v4.1.0
Browser Version: n/a

@subzero10
Copy link
Member Author

subzero10 commented Sep 16, 2022

This happens because we wanted to keep the same lambda handler definition as the handler we are wrapping:

  • for async / promise-based handlers, we wrap them to a promise-based handler
  • for callback-based handlers, we wrap them to a callback-based handler

In order to achieve this, we have to deduce the type of the handler and the way we do that is not 100% consistent: we simply check the number of parameters of the handler. If there are more than two, we assume it's a callback-based handler (with event, context and callback) otherwise we assume it's an async/promise-based handler. This check though does not cover the case where we have a synchronous handler that returns immediately (i.e. parameters are 2 or less and it does not return a promise).
This PR handles this case and wraps these handlers into promise-based handlers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
js @honeybadger-io/js
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant