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

Jetty 12: Review asyncError handling #11079

Closed
sbordet opened this issue Dec 17, 2023 · 1 comment
Closed

Jetty 12: Review asyncError handling #11079

sbordet opened this issue Dec 17, 2023 · 1 comment
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@sbordet
Copy link
Contributor

sbordet commented Dec 17, 2023

Jetty version(s)
12+

Jetty Environment
ee10

Description
After #10949, ServletChannelState.asyncError() is now invoked as part of calls to onFailure() since it is invoked through a failure listener.

However, I think this is a bit too much, as asyncError() was initially designed for client-side errors delivered via HTTP/2 RST_STREAM.

Now, since it's invoked via the onFailure() mechanism, it is invoked also in many other cases, often producing IllegalStateException -- a signal that it may not be right to invoke it.

We should review whether restore the original semantic for asyncError() (only invoked for RST_STREAM), or modify it for the new one (always invoked for onFailure()).

@sbordet
Copy link
Contributor Author

sbordet commented Nov 21, 2024

This has been addressed.

It was deemed ok to always invoked asyncError() in case of asynchronous failures, of which HTTP/2 RST_STREAM is only one case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
Status: ✅ Done
Development

No branches or pull requests

1 participant