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

apollo-server-lambda: createHandler's handler doesn't take a callback #5188

Merged
merged 1 commit into from
May 7, 2021

Conversation

glasser
Copy link
Member

@glasser glasser commented May 7, 2021

Lambda has two ways of defining a handler: as an async Promise-returning
function, and as a callback-invoking function.
https://docs.aws.amazon.com/lambda/latest/dg/nodejs-handler.html

Until v2.21.2 our handler was only a callback-invoking function.
Starting to that version, it could be called in either way. This meant
that the TypeScript types for our handler were somewhat vague (see
#5072).

All current Node Lambda runtimes support calling async functions; the
only reason that we didn't just change directly from callback-invoking
to async in v2.21.2 was to support the use case of user code that wraps
our handler in their own function. But now we're doing AS3 so we can
make backwards-incompatible changes, so if you're doing that, you should
just make your handler async too and await the result of our handler.

Fixes #5018.

@glasser glasser mentioned this pull request May 7, 2021
@glasser glasser requested a review from abernix May 7, 2021 17:35
@glasser
Copy link
Member Author

glasser commented May 7, 2021

Post-merge review if you'd like (pretty straightforward though)

@glasser glasser enabled auto-merge (squash) May 7, 2021 17:35
Lambda has two ways of defining a handler: as an async Promise-returning
function, and as a callback-invoking function.
https://docs.aws.amazon.com/lambda/latest/dg/nodejs-handler.html

Until v2.21.2 our handler was only a callback-invoking function.
Starting to that version, it could be called in either way. This meant
that the TypeScript types for our handler were somewhat vague (see
#5072).

All current Node Lambda runtimes support calling async functions; the
only reason that we didn't just change directly from callback-invoking
to async in v2.21.2 was to support the use case of user code that wraps
our handler in their own function. But now we're doing AS3 so we can
make backwards-incompatible changes, so if you're doing that, you should
just make your handler async too and `await` the result of our handler.

Fixes #5018.
@glasser glasser force-pushed the glasser/lambda-no-maybecallbackify branch from 55b1ec8 to cc8a622 Compare May 7, 2021 18:02
@glasser glasser merged commit 8382b61 into release-3.0 May 7, 2021
@glasser glasser deleted the glasser/lambda-no-maybecallbackify branch May 7, 2021 18:05
Copy link
Member

@abernix abernix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants