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

Fix StatisticsHandler in the case a Handler throws exception. #7837

Merged
merged 1 commit into from
May 4, 2022

Conversation

lachlan-roberts
Copy link
Contributor

If a Handler throws, the StatisticsHandler thinks it has seen a 200 response because the status code has not yet been set. This PR adds a separate stat _responsesThrown which is now incremented instead of using the incorrect status code in the case the handler throws.

@joakime
Copy link
Contributor

joakime commented Apr 5, 2022

Having a late point of status tracking, to supplement the tracking in the StatisticsHandler is a good idea.
Maybe having a stats specific HttpChannel.Listener or stats RequestLog that forwards the status codes to the tracking in StatisticsHandler is in our future.

@gregw
Copy link
Contributor

gregw commented Apr 5, 2022

Note that we have a late stats point in jetty-12, but I don't think we should make significant changes in <= 11

@joakime joakime added the Bug For general bugs on Jetty side label Apr 26, 2022
@joakime
Copy link
Contributor

joakime commented Apr 26, 2022

@lachlan-roberts is there an open issue for this?

@lachlan-roberts
Copy link
Contributor Author

@joakime yes there is an open private issue which has been linked.

@lachlan-roberts lachlan-roberts merged commit 125e12a into jetty-9.4.x May 4, 2022
@lachlan-roberts lachlan-roberts deleted the jetty-9.4.x-statsHandlerThrows branch May 4, 2022 05:59
joakime added a commit that referenced this pull request May 4, 2022
Signed-off-by: Joakim Erdfelt <[email protected]>
lachlan-roberts added a commit that referenced this pull request May 6, 2022
Issue #7837 - Fix StatisticsHandler in the case a Handler throws exception.
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants