search

vert-x3 / vertx-web

HTTP web applications for Vert.x
Apache License 2.0
1.11k stars 535 forks source link

fix: report client closure to request promise #2631

Closed zekronium closed 4 weeks ago

zekronium commented 4 months ago

Motivation:

If the client is closed (or other exception is thrown), it wont be handled and instead will be thrown on the context exception handler.

Normally this is fine, but in VIRTUAL_THREAD threading mode it causes threads to never be awaken.

Note: this technically can be solved by also changing how the client.request throws its exceptions and to fail the request promise too, but here it seems more appropriate in my eyes.

tsegismont commented 4 months ago

Thanks for this PR. Is there a corresponding issue?

Can you please add a test that does not pass with this change?

cc @vietj

vietj commented 4 months ago

yes I think the request method should not fail synchronously

zekronium commented 4 months ago

yes I think the request method should not fail synchronously

Are we resulting then to failing it inside vertx-core then?

zekronium commented 4 weeks ago

@vietj This is quite key for virtual threads. You think its good?

vietj commented 4 weeks ago

can you provide a test failing that ? or a stack trace ?

zekronium commented 4 weeks ago

can you provide a test failing that ? or a stack trace ?

I think this naturally fixed itself with the rework of TaskQueue. Can not reproduce it the same way as it was before

vietj commented 4 weeks ago

ok then I'll close this issue until we have a reproducer