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

Atomic replacement of existing request #84

Open
fergald opened this issue Oct 5, 2023 · 1 comment
Open

Atomic replacement of existing request #84

fergald opened this issue Oct 5, 2023 · 1 comment

Comments

@fergald
Copy link
Collaborator

fergald commented Oct 5, 2023

Problem

Our current design allows a request to be cancelled. It was argued that there was no need for

  • atomic update of a request
  • a sendNow method

as these could both be implemented in JS. While it's true that they can be implemented, they are vulnerable to data loss if a crash occurs. E.g. the sample code to update a beacon is

let beaconResult = null;
let beaconAbort = null;

function updateBeacon(data) {
  const pending = !beaconResult || !beaconResult.sent;
  if (pending && beaconAbort) {
    beaconAbort.abort();
  }

  createBeacon(data);
}

function createBeacon(data) {
  if (beaconResult && beaconResult.sent) {
    // Avoid creating duplicated beacon if the previous one is still pending.
    return;
  }

  beaconAbort = new AbortController();
  beaconResult = fetchLater({
    url: data
    signal: beaconAbort.signal
  });
}

If the process running JS dies between abort and createBeacon then it's possible that the old request will be aborted but no new request will be created.

A similar problem exists when devs abort a fetchLater and do an immediate fetch. If a death occurs, between these then it's possible that neither request will actually be sent.

Severity

Using Chrome (and I believe WebKit) terminology, JS is running in the renderer, fetchLater requests are managed by the browser.

The window in which the death must occur is varies by case

  • replace request: the death must occur after the message to abort has been received by the browser but before the message to create a new fetch has been received by the browser
  • immediate send: the death must occur after the message to abort has been received by the browser but before all of the data for the fetch has been received by whatever process handles network data

The immediate send case may be considerably longer but given that deaths are fairly rare (if JS is running then we are likely not in the background etc).

@fergald
Copy link
Collaborator Author

fergald commented Oct 16, 2023

I'm just going to leave this here for the record, I want to show that the current API could easily be extended to give atomic operations. These are just straw-person APIs

replaces option

Add a replaces option to the fetch options. The replaces option gives a handle to an existing pending fetch later that should be cancelled in favour of the new request. It would be accepted by fetchLater and fetch.

So you can do something like

let laterHandle = fetchLater({
    url: data
});
...
// Now we want to update the fetch. This cancels the old one and queues a new one
// atomically.
laterHandle = fetchlater({
    url: newData,
    replaces: laterHandle,
});

// Now we want to do an immediate send of a pending fetch. This cancels the old one and immediately starts a new one
// atomically.
let fetchHandle = fetch({
    replaces: laterHandle,
});

Maybe when switching to fetch, it keeps all of the attributes of the old fetchLater including it's keep-alive nature.
There is some question over whether it would be allowed to set new options in that call. The point is that making these operations atomic does not require a breaking change in the API.

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

No branches or pull requests

1 participant