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

HttpClientConnection catches failures more broadly #547

Merged

Conversation

carterkozak
Copy link
Contributor

In some (likely failure) cases I've noticed the undertow client
fails to execute any ClientCallback methods.

I've noticed NPEs in the the logs from SslConduit.doUnwrap, so
it's possible this is caused by UNDERTOW-1156.

@carterkozak
Copy link
Contributor Author

This change is a bit paranoid, feel free to close out if you think the problem is likely elsewhere.

@undertow-pull-request
Copy link

Windows Build 2168 outcome was FAILURE using a merge of c5790bb
Summary: Tests passed: 2881, ignored: 618; exit code 1 Build time: 00:38:00

private HttpClientExchange currentRequest;
private HttpResponseBuilder pendingResponse;
private volatile HttpClientExchange currentRequest;
private volatile HttpResponseBuilder pendingResponse;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the volatile? This client is not designed to be thread safe, and should old be used from the IO thread that is tied to the connection.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was paranoia. Fixed.

In some (likely failure) cases I've noticed the undertow client
fails to execute any ClientCallback methods.

I've noticed NPEs in the the logs from SslConduit.doUnwrap, so
it's possible this is caused by UNDERTOW-1156.
@undertow-pull-request
Copy link

Windows Build 2169 outcome was FAILURE using a merge of 640e3ce
Summary: Tests passed: 2881, ignored: 618; exit code 1 Build time: 00:28:05

@carterkozak
Copy link
Contributor Author

carterkozak commented Aug 24, 2017

Once 1.4.19.Final has published I'll run my test suite against that and report whether or not the hang is fixed.
It looks like the version is tagged but unpublished. Looks like it's available now

@carterkozak
Copy link
Contributor Author

Still having requests fail to hit the callback on 1.4.19.Final using http1.1. The If I force h2c-prior/h2 protocols instead flakes are considerably less common (haven't seen any since upgrading to 1.4.19).

@carterkozak
Copy link
Contributor Author

Added a close setter on checkout which triggers a callback as well, but that doesn't seem to do it either.

For context, I've set up timers on my connections which kill the request after ten minutes if nothing has happened.

The entirety of my request data is sent, but the response callback is never called.

@stuartwdouglas stuartwdouglas merged commit 1842b27 into undertow-io:master Aug 24, 2017
@stuartwdouglas
Copy link
Contributor

Do you have any information on which exceptions are being thrown from these methods?

@carterkozak
Copy link
Contributor Author

@stuartwdouglas Unfortunately I've not been able to get any logging around the cause

@carterkozak
Copy link
Contributor Author

@stuartwdouglas After some digging it appears that the hang occurs (at very low frequency) when clients attempt to connect to servers that are in the process of starting up. Likely some unhandled failure when unexpected data is received.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants