-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Re-enable HttpChannelEventTest and port to ee9 #12477
base: jetty-12.1.x
Are you sure you want to change the base?
Re-enable HttpChannelEventTest and port to ee9 #12477
Conversation
connector.addBean(new HttpChannel.Listener() | ||
{ | ||
@Override | ||
public void onRequestFailure(Request request, Throwable failure) | ||
{ | ||
//TODO never called |
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.
The onRequestFailure and onComplete methods will never be called for a bad request in jetty-12, as that is determined in core and thus we do not know what context the bad request would have gone to.
So this test is invalid and we should update the javadoc of the listener class to say as much
connector.addBean(new HttpChannel.Listener() | ||
{ | ||
@Override | ||
public void onResponseFailure(Request request, Throwable failure) | ||
{ | ||
//TODO this is never called |
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.
This could be a bug. I see no reason that a response failure within a context cannot be notified like this.
Further analysis required.
Port
HttpChannelEventTest
toee9
and re-enable it.