Skip to content

Disallow synchronous handlers in type definition #308

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

Merged
merged 2 commits into from
Feb 15, 2022
Merged

Conversation

Pimm
Copy link
Contributor

@Pimm Pimm commented Jan 16, 2022

Which problem is this pull request solving?

The type definition of Handler (or BaseHandler, more specifically) allows handlers to by synchronous.

I wrote some Netlify functions, which worked as expected locally through Netlify CLI. On production, however, the function produced the following:

error decoding lambda response: invalid status code returned from lambda: 0

This seems to have been caused by the functions returning a Response (directly) rather than a Promise<Response> in some branches. Synchronous handlers are allowed by (Netlify CLI, as well as) the type definition, but not actually by Netlify.

List other issues or pull requests related to this problem

(none that I could find)

Describe the solution you've chosen

Removed the synchronous version from the union that is the return type of BaseHandler.

Describe alternatives you've considered

Add support for synchronous handlers to the Netlify platform.

Checklist

  • I have read the contribution guidelines.
  • The status checks are successful (continuous integration). Those can be seen below.

Removed the synchronous version from the union that is the return type of BaseHandler.
@Pimm Pimm changed the title Disallow synchronous handlers in type definition fix: Disallow synchronous handlers in type definition Jan 23, 2022
@Pimm Pimm changed the title fix: Disallow synchronous handlers in type definition Disallow synchronous handlers in type definition Jan 23, 2022
@minivan minivan self-assigned this Jan 28, 2022
@minivan
Copy link

minivan commented Jan 28, 2022

Thank you for this, Pimm! This is great, we'll check if it breaks anything on our end and get back to this PR 🙌

@minivan minivan assigned Skn0tt and unassigned minivan Feb 14, 2022
Skn0tt
Skn0tt previously approved these changes Feb 15, 2022
@Skn0tt Skn0tt added the type: bug code to address defects in shipped code label Feb 15, 2022
Copy link
Contributor

@Skn0tt Skn0tt left a comment

Choose a reason for hiding this comment

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

LGTM! I agree with your reasoning. The fact that ntl dev supports it, I consider a bug in itself. I'll see what we can do to mirror Lambda's behaviour in ntl dev as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants