Skip to content

fix: raise an error during wizening for async functions given to addEventListener #689

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 1 commit into from
Oct 16, 2023

Conversation

JakeChampion
Copy link
Contributor

resolves #687

Below is what what it looks like if you try and register an async fetchevent handler

❯ fastly compute serve
✓ Verifying fastly.toml
✓ Identifying package name
✓ Identifying toolchain
✗ Running [scripts.build]

INFO: Command output:

Error: addEventListener: Argument 2 should not be async function. Please rewrite this to be a synchronous function which returns a Response or a Promise of a Response.
Error: the wizer.initialize function trapped

The fetch event handler function has to be sync and not async because the Event Model itself is synchronous and you can register multiple FetchEventHandlers.
If you pass an async function as the FetchEventHandler and await before your call to event.responseWith - then the FetchEventHandler will return a Promise, when the FetchEventHandler returns anything (step 24.3.12 of Handle Fetch algorithm), the specification states we need to check if the the respond-with flag has been set, and it would not have been set - we then need to call the next registered FetchEventHandler - doing the same steps, until one calls event.respondWith or we have called them all.

This thread has some more context: w3c/ServiceWorker#836

@JakeChampion JakeChampion requested a review from elliottt October 13, 2023 23:47
@JakeChampion JakeChampion force-pushed the jake/async-fetch-event-error-message branch from 687f422 to 1bfde2f Compare October 13, 2023 23:54
@JakeChampion JakeChampion force-pushed the jake/async-fetch-event-error-message branch from 1bfde2f to 1ec52b8 Compare October 14, 2023 00:44
@JakeChampion JakeChampion requested a review from elliottt October 14, 2023 00:44
@JakeChampion JakeChampion force-pushed the jake/async-fetch-event-error-message branch from 1ec52b8 to d6e97a7 Compare October 16, 2023 10:51
Copy link
Contributor

@elliottt elliottt left a comment

Choose a reason for hiding this comment

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

Awesome!

@elliottt elliottt force-pushed the jake/async-fetch-event-error-message branch from d6e97a7 to a1734fd Compare October 16, 2023 17:38
@elliottt elliottt enabled auto-merge (squash) October 16, 2023 17:39
@elliottt elliottt merged commit e6747a2 into main Oct 16, 2023
@elliottt elliottt deleted the jake/async-fetch-event-error-message branch October 16, 2023 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Raise an error during wizening for async functions given to addEventListener
2 participants