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

Support cancellation of HTTP requests #712

Open
DerGuteMoritz opened this issue Feb 12, 2024 · 2 comments · Fixed by #714
Open

Support cancellation of HTTP requests #712

DerGuteMoritz opened this issue Feb 12, 2024 · 2 comments · Fixed by #714

Comments

@DerGuteMoritz
Copy link
Collaborator

DerGuteMoritz commented Feb 12, 2024

Problem

At the moment it is not possible to cancel HTTP requests (be it in-flight or during connection establishment). The only option users have is to just drop the response deferred on the floor and have it GC'ed. However, this doesn't free up the underlying connection until the response is complete. Combined with the fact that there is no default request-timeout, this can lead to resource exhaustion.

Manifold's idiom for cancelling a deferred is to put it into an error state but that only affects chained deferreds downstream from the "cancelled" one (see also clj-commons/manifold#167). Since the response deferred returned by aleph.http/request sits at the very end of the deferred chain, putting it into an error state has no repercussion on any of the upstream deferreds nor on the underlying connection.

This also means that placing a timeout on a response deferred to limit the overall request duration (i.e. connection setup, request transmission and response reception) doesn't have the effect a user might expect:

@(-> (http/get "...") (d/timeout! 1000))

As explained above, this will simply put the response deferred into an error state when the timeout expires but the underlying request operation will still proceed, tying up any resources it has acquired until done.

Solutions

The most elegant solution would be to change Manifold to also cancel any upstream deferreds which feed into a "cancelled" deferred (if there are no others left). However, according to Zach, this cannot be implemented in the current architecture of Manifold and would require a "major reengineering".

A more tractable solution would be to explicitly cancel any upstream operations when the response deferred is set into an error state.

@DerGuteMoritz
Copy link
Collaborator Author

Related old issue where a user attempted to use d/timeout! as in the example above: #152

DerGuteMoritz added a commit that referenced this issue Feb 12, 2024
This patch changes `aleph.http/request` so that setting the response deferred to an error status
will terminate an in-flight request.

Closes #712.
DerGuteMoritz added a commit that referenced this issue Feb 12, 2024
This patch changes `aleph.http/request` so that setting the response deferred to an error status
will terminate an in-flight request. This allows e.g. for `d/timeout!` to be used without
potentially leaking connections.

For convenient explicit cancellation, we provide `aleph.http/cancel-request!`. IT sets the given
response deferred to error with an instance of the new `aleph.utils.RequestCancellationException`.

Closes #712.
DerGuteMoritz added a commit that referenced this issue Feb 12, 2024
This patch changes `aleph.http/request` so that setting the response deferred to an error status
will terminate an in-flight request. This allows e.g. for `d/timeout!` to be used without
potentially leaking connections.

For convenient explicit cancellation, we provide `aleph.http/cancel-request!`. It sets the given
response deferred to error with an instance of the new `aleph.utils.RequestCancellationException`.

Closes #712.
DerGuteMoritz added a commit that referenced this issue Feb 12, 2024
This patch changes `aleph.http/request` so that setting the response deferred to an error status
will terminate an in-flight request. This allows e.g. for `d/timeout!` to be used without
potentially leaking connections.

For convenient explicit cancellation, we provide `aleph.http/cancel-request!`. It sets the given
response deferred to error with an instance of the new `aleph.utils.RequestCancellationException`.

Closes #712.
DerGuteMoritz added a commit that referenced this issue Feb 12, 2024
This patch changes `aleph.http/request` so that setting the response deferred to an error status
will terminate an in-flight request. This allows e.g. for `d/timeout!` to be used without
potentially leaking connections.

For convenient explicit cancellation, we provide `aleph.http/cancel-request!`. It sets the given
response deferred to error with an instance of the new `aleph.utils.RequestCancellationException`.

Closes #712.
DerGuteMoritz added a commit that referenced this issue Feb 12, 2024
This patch changes `aleph.http/request` so that setting the response deferred to an error status
will terminate an in-flight request. This allows e.g. for `d/timeout!` to be used without
potentially leaking connections.

For convenient explicit cancellation, we provide `aleph.http/cancel-request!`. It sets the given
response deferred to error with an instance of the new `aleph.utils.RequestCancellationException`.

Closes #712.
DerGuteMoritz added a commit that referenced this issue Feb 14, 2024
This patch changes `aleph.http/request` so that setting the response deferred to an error status
will terminate an in-flight request. This allows e.g. for `d/timeout!` to be used without
potentially leaking connections.

For convenient explicit cancellation, we provide `aleph.http/cancel-request!`. It sets the given
response deferred to error with an instance of the new `aleph.utils.RequestCancellationException`.

Closes #712.
DerGuteMoritz added a commit that referenced this issue Mar 12, 2024
This patch changes `aleph.http/request` so that setting the response deferred to an error status
will terminate an in-flight request. This allows e.g. for `d/timeout!` to be used without
potentially leaking connections.

For convenient explicit cancellation, we provide `aleph.http/cancel-request!`. It sets the given
response deferred to error with an instance of the new `aleph.utils.RequestCancellationException`.

Closes #712.
DerGuteMoritz added a commit that referenced this issue Mar 12, 2024
This patch changes `aleph.http/request` so that setting the response deferred to an error status
will terminate an in-flight request. This allows e.g. for `d/timeout!` to be used without
potentially leaking connections.

For convenient explicit cancellation, we provide `aleph.http/cancel-request!`. It sets the given
response deferred to error with an instance of the new `aleph.utils.RequestCancellationException`.

Closes #712.
@DerGuteMoritz
Copy link
Collaborator Author

Cancellation of in-flight connection attempts is still missing. This can be implemented once Netty 4.1.108.Final is released which will include the patch for fixing netty/netty#13843.

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

Successfully merging a pull request may close this issue.

1 participant