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

Jetty9: unhandled response should be counted as 404 and not 200 #1232

Merged
merged 1 commit into from Dec 19, 2017

Conversation

Projects
None yet
2 participants
@swallez

swallez commented Dec 18, 2017

In Jetty, a request that has not been handled by the handler chain keeps the default status code 200, and a 404 response is produced by HttpChannel. The Jetty9 InstrumentedHandler therefore has to count unhandled request as 404 and not 200.

To reveal the issue, this PR removes the 404 response from InstrumentedHandlerTest.TestHandler so that a request to /hello ends up not being handled.

This PR is against branch 3.2-development. Should I open another PR for branch 4.0-development?

Jetty9: unhandled response should be counted as 404 and not 200
In Jetty, a request that has not been handled by the handler chain keeps the
default status code 200, and a 404 response is produced by HttpChannel. The
Jetty9 InstrumentedHandler therefore has to count unhandled request as 404.
@arteam

This comment has been minimized.

Member

arteam commented Dec 19, 2017

Thanks for spotting this bug! I agree that an unhandled request should be counted as an error (because there's no handler that handles the request).

I'm still maintaining the 3.2 branch, so the fix is good enough to merge to it. I will also cherry-pick it to the 4.0 branch.

@arteam arteam merged commit 68087f9 into dropwizard:3.2-development Dec 19, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@arteam arteam added the bug label Dec 19, 2017

@arteam arteam added this to the 3.2.6 milestone Dec 19, 2017

@swallez

This comment has been minimized.

swallez commented Dec 19, 2017

Awesome! Thanks @arteam!

arteam added a commit that referenced this pull request Jan 5, 2018

Jetty9: unhandled response should be counted as 404 and not 200 (#1232)
In Jetty, a request that has not been handled by the handler chain keeps the
default status code 200, and a 404 response is produced by HttpChannel. The
Jetty9 InstrumentedHandler therefore has to count unhandled request as 404.

(cherry picked from commit 68087f9)

arteam added a commit that referenced this pull request Jan 5, 2018

Jetty9: unhandled response should be counted as 404 and not 200 (#1232)
In Jetty, a request that has not been handled by the handler chain keeps the
default status code 200, and a 404 response is produced by HttpChannel. The
Jetty9 InstrumentedHandler therefore has to count unhandled request as 404.

(cherry picked from commit 68087f9)
(cherry picked from commit 5481990)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment