-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add timeouthandler #310
Add timeouthandler #310
Conversation
Wraps around Vert.x TimeoutHandler implementation to write a 500 response if a request is not completed by another handler within 15 seconds Fixes cryostatio#288
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.
Some handlers, like the handlers to create a recording or view recordings present in a target, don't timeout even with a 1ms timeout. Is that okay/expected?
Also, needs a spotless format.
src/main/java/com/redhat/rhjmc/containerjfr/net/web/http/generic/TimeoutHandler.java
Outdated
Show resolved
Hide resolved
Hmm. I'm not sure why that would be - I've just wrapped around the Vertx standard timeout handler implementation and added it to the handler chain. Maybe whatever asynchronous monitor is watching for responses to be written to the context doesn't have the timing resolution to be so strict as to reject with timeouts as short as 1ms. Or perhaps we're blocking the event loop on those handlers and so the monitor doesn't actually get processed until after our handler executes - in which case, we need to change the The backing implementation is here but it doesn't provide much insight: Here's a piece of doc which references this https://vertx.io/docs/vertx-core/java/#_executing_periodic_and_delayed_actions Anyway, the main use case here is to ensure that clients don't get left hanging if:
All of these are cases where a long timeout of 5+ seconds should be easily caught by whatever async piece is monitoring us under Vertx's hood. |
* Add timeouthandler Wraps around Vert.x TimeoutHandler implementation to write a 500 response if a request is not completed by another handler within 15 seconds Fixes cryostatio#288 * Add 'L' to long literal * Apply Spotless formatting
…yostatio#310) Bumps [io.cryostat:cryostat-core](https://github.com/cryostatio/cryostat-core) from 2.29.1 to 2.30.0. - [Release notes](https://github.com/cryostatio/cryostat-core/releases) - [Commits](cryostatio/cryostat-core@v2.29.1...v2.30.0) --- updated-dependencies: - dependency-name: io.cryostat:cryostat-core dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Wraps around Vert.x TimeoutHandler implementation to write a 500 response if a request is not completed by another handler within 15 seconds
Fixes #288
Easy ways to test: modify the
15_000
constant in the patch to a shorter period, like500
, and then do some operation like generating a report. Or, build a minimal image and try to request a web-client asset.