HTTP Request Cancellation Semantics

[teliosdev]

I'm not sure if this is the right place for this. I think I've found a bug in the fetch implementation provided by Tauri in regards to the request cancellation for the http Tauri plugin.

As a part of the fetch standard, one can pass in an optional AbortSignal for aborting the active request and response. The standard specifies:

To abort a fetch() call with a promise, request, responseObject, and an error:

1. Reject promise with error. (Note: this is a no-op if promise has already fulfilled.)

However, ability of the Tauri implementation of fetch to do so is at best dependent on a race condition, and at worst unable to do so.

Starting from the JavaScript client, the fetch method performs two invocations; the first being plugin:http|fetch, to construct the request and send it over the network, and plugin:http|fetch_send to poll the result. The former returns the resource ID associated with the request, and the latter consumes the resource ID, making it unavailable by taking it from the table.

However, there is a third invocation in the case of a cancellation - when a cancellation signal is sent, the plugin:http|fetch_cancel invocation is made, using the resource ID provided by plugin:http|fetch. However, that resource ID is invalidated at the start of the plugin:http|fetch_send invocation, meaning that the window for fetch_cancel to actually operate on that resource is very narrow - between the addition of the event listener to the signal, and the start invocation of the fetch_send. Depending on how the JavaScript schedules the respective requests, the fetch_cancel invocation will never be able to work as expected, as fetch_send will have pre-empted it. There is a third case - if the request is aborted immediately after fetch without awaiting on the results of fetch, on the same tick, the abort signal will be silently ignored, since the event listener is only added after fetch is called.

So to sum, the abort signal will do one of the following: (1) nothing, as it's silently ignored; (2) fail to cancel the request (the most common outcome); or (3) cancel the request, but not necessarily the response.

I'm not certain how closely Tauri wants to follow the WHATWG fetch specification, but given that Tauri has added the API to abort an active HTTP request, it would make sense to classify this as a bug.

I'm not sure how to fix this, as the core of the issue seems to be that the request is removed from the resource table before the cancel can act upon it, and then indefinitely awaited for the result. The easiest way I think you could do this is to add a tokio watch as a latch as another field within the FetchRequest; fetch_send would not take the resource from the table until after the request has been finished (or the latch triggers). Then, when awaiting the request, it can be raced with waiting until the latch triggers or errors. All fetch_cancel needs to do is trigger the latch; it does not need to acquire the lock. (This does mean that before awaiting the response, fetch_send would need to check the latch to make sure it should not immediately abort.) The only problem, then, is aborting the reading; but if the latch is passed through to the RequestResponse, we can abort the response (if the command is added and the code is added to the JavaScript client).

I'd be happy to make the pull request if that's the desired path, but as I'm not familiar with this project, I thought an issue would be better.

Reproduction

Reproducing the bug is as simple as this:

import { fetch } from "@tauri-apps/plugin-http";

async function abortingRequest() {
  const abort = new AbortController();
  // `BAD_ADDRESS` should be a URL that, for testing purposes, does not
  // respond immediately.
  const promise = fetch(`http://${BAD_ADDRESS}`, { signal: abort.signal });
  // if you abort immediately, with the commented out code, the abort does
  // nothing.
  //abort.abort();
  // Or, set a delay...
  setTimeout(() => abort.abort(), 1);
  // the following line should throw an exception, but it does not.
  const response = await promise;
}

Full tauri info output

[✔] Environment
    - OS: Mac OS 13.6.6 X64
    ✔ Xcode Command Line Tools: installed
    ✔ rustc: 1.78.0 (9b00956e5 2024-04-29)
    ✔ cargo: 1.78.0 (54d8815d0 2024-03-26)
    ✔ rustup: 1.27.1 (54dd3d00f 2024-04-24)
    ✔ Rust toolchain: stable-aarch64-apple-darwin (default)
    - node: 18.16.0
    - pnpm: 9.0.6
    - npm: 9.5.1

[-] Packages
    - tauri [RUST]: 2.0.0-beta.20
    - tauri-build [RUST]: 2.0.0-beta.16
    - wry [RUST]: 0.40.0
    - tao [RUST]: 0.28.0
    - @tauri-apps/api [NPM]: 2.0.0-beta.12
    - @tauri-apps/cli [NPM]: 2.0.0-beta.18

[-] App
    - build-type: bundle
    - CSP: unset
    - frontendDist: ../dist
    - devUrl: http://localhost:1420/
    - bundler: Vite

[amrbashir]

Could you try #1395 and see if it fixes some of the issue you have?

unfortunately, I don't think we can easily fix aborting immediately, because we need to get an Id from the backend and that's an async task, so by the time you call abort, the ID hasn't been returned and thus aborting will do nothing.

However, with #1395, the window to cancel, should be a lot flexible.

[teliosdev]

1395 definitely fixes part of the issue, but not the whole issue; I believe if you insert a check to see if the signal is aborted (e.g., if(signal?.aborted)) after installing the signal handler, you can preemptively cancel the request before even having to send the plugin:http|fetch_send invocation, solving the rest of the problem.

You can also avoid mutexes entirely in the commands by using the tokio::sync::watch channel (though you'll need to check the value of the watch in a similar way to the signal abort before the select!), if you want.

[amrbashir]

I believe if you insert a check to see if the signal is aborted (e.g., if(signal?.aborted)) after installing the signal handler, you can preemptively cancel the request before even having to send the plugin:http|fetch_send invocation, solving the rest of the problem.

That's a nice suggestion, I think it solved the remaining issue. Pushed the changes to the PR if you want to test it.

You can also avoid mutexes entirely in the commands by using the tokio::sync::watch channel (though you'll need to check the value of the watch in a similar way to the signal abort before the select!), if you want.

Thanks, I was using mutexes for a quick way to test things but I will definitely look for a more efficient approach.

[amrbashir]

Pushed another commit to remove the two mutexes I originally added. There is still one mutex that always existed before that, I may try to remove that in the future, but it should be fine for now.

[teliosdev]

Sorry for the late reply, but #1395 fixed it entirely for me!

[amrbashir]

No need to apologize @teliosdev , thanks for testing and for the great issue as well.