-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
chore(ext/http): add "legacy_abort" feature flag #28371
chore(ext/http): add "legacy_abort" feature flag #28371
Conversation
Fixes denoland#27005
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add feature flag, global set on startup
- test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a runtime feature flag is better
/// If `false`, the server will abort the request when the response is dropped. | ||
pub no_legacy_abort: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is this set to true? Ideally it would be an unstable feature flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Please land with a descriptive commit message about the feature flag and default value and the fact that this is a breaking change and hence we're being careful about it.
Deno.serve `Request` abort signals are aborted by default even when it is finished successfully. This PR gates this behavior behind the "legacy_abort" which is the default right now. Turning the `no_legacy_abort` runtime option on is a **breaking change** and will only abort request signals when there is a failure, thereby cannot be used to determine if the request finished. This aligns with `fetch` API. Ref #27005
Deno.serve
Request
abort signals are aborted by default even when it is finished successfully. This PR gates this behavior behind the "legacy_abort" which is the default right now.Turning the
no_legacy_abort
runtime option on is a breaking change and will only abort request signals when there is a failure, thereby cannot be used to determine if the request finished. This aligns withfetch
API.Fixes #27005