Feature/add cancellation support#1510
Conversation
|
|
||
| 3. **Error Handling**: Cancelled operations will complete with an `OperationException` containing a `CancelledException` as the `linkException`. | ||
|
|
||
| 4. **Network Cleanup**: Cancelling an operation will attempt to cancel the underlying network request, but depending on the transport layer and server, the request may still complete on the server side. |
There was a problem hiding this comment.
can't really see how it cancels the underlying request? can you explain?
There was a problem hiding this comment.
you are right i haven't noticed that the underlying http request wasn't actually cancelled, only the subscription was cancelled.
I now added true HTTP request cancellation to the graphql package.
-
Created custom HttpLink with platform-specific cancellation support
-
Web: Uses XMLHttpRequest.abort() → shows "(canceled)" in Chrome DevTools
-
IO (Linux/Desktop/Mobile): Uses HttpClient.close(force: true) + request.abort() → terminates TCP connection
-
Added CancellationContextEntry to pass the token through the Link chain
-
Added web package dependency (replaces deprecated dart:html)
-
Added additional tests for http cancellation
Result:
operation.cancel() now actually aborts the underlying HTTP request
Not just ignoring the response — the network connection is terminated
I verified it, in chrome and linux the http request gets actually cancelled
|
|
||
| MutationOptions<TParsed> copyWithPolicies(Policies policies) => | ||
| MutationOptions( | ||
| MutationOptions<TParsed> copyWithPolicies(Policies policies) => MutationOptions( |
There was a problem hiding this comment.
You should run dart format --line-length=80 ./
There is quite a lot of irrelevant white space noise in this PR
|
Hi thanks for the contrinbution, this PR is not getting any review because I was not pinged by my internal review bot due that commits are not following the contribution guidelines. I suggest you add it to the claude.md or any other agent.md file because following the commit style is what get any PR merged in a couple of hours or delayed months. Sorry for commit style see https://github.com/zino-hofmann/graphql-flutter/blob/main/docs/dev/MAINTAINERS.md#commit-style |
8a0d116 to
e4e05cf
Compare
|
I'm very skeptical of this approach. It does not seem to consider how this would work with observables. It also does not really play well with the cache; any cancellation that does not know about the link and the cache will not work in a nice way when you throw streams into the mix. It also makes me very skeptical because, when I looked at the original PR, it did not actually do what it claimed it did, which makes me think "AI slop" right away. |
As needed since 6 years here is cancellation support #181
-> no breaking changes
-> all tests pass