emscripten icon indicating copy to clipboard operation
emscripten copied to clipboard

Use Fetch in preference of XMLHttpRequest (on supported platforms)

Open LPardue opened this issue 8 years ago • 14 comments

Background

I wrote up some background on the emscripten-discuss group here.

To summarise, src/shell.js contains some calls to XMLHttpRequest (XHR). The Fetch API is a replacement for XHR, that has quite widespread support (https://caniuse.com/#search=fetch). Service Worker does not support XHR in Chrome or Firefox (I have not tested other browsers).

Affected files

src/shell.js

Proposed solution

Use Fetch in preference to XHR on supported platforms.

Known/potential problems

Fetch does not support the File URI scheme (ticket). Further analysis is required to understand how this would affect emscripten. XHR might be required as a fallback.

LPardue avatar Feb 13 '18 15:02 LPardue

Out of curiosity, has anything happened regarding this issue?

mikaelast avatar Nov 20 '18 14:11 mikaelast

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

stale[bot] avatar Nov 20 '19 15:11 stale[bot]

This should not be closed. The generated JS file is littered with XMLHttpRequest which should be replaced with fetch. E.g. Deno does not support XMLHttpRequest but it does fetch.

https://github.com/denoland/deno/issues/2191

Ciantic avatar Jun 15 '22 23:06 Ciantic

I got Deno working with simple sed tool:

sed -i 's|\(readAsync\s*=\s*(url,\s*onload,\s*onerror)\s*=>\s*{\)|\1fetch(url).then(async response => { onload(await response.arrayBuffer());}).catch(onerror); return;|g' YOURFILE.js

I also tested patching this

https://github.com/emscripten-core/emscripten/blob/c0f1058cc53736a078e1bfaa42f0f2b8beebbccb/src/web_or_worker_shell_read.js#L48-L68

With this:

  readAsync = (url, onload, onerror) => {
    #if SUPPORT_BASE64_EMBEDDING
      var data = tryParseAsDataURI(url);
      if (data) {
        onload(data.buffer);
        return;
      }
    #endif

    fetch(url).then(response => { 
      response.arrayBuffer().then(onload).catch(onerror)
    }).catch(onerror);
  }

And it works.

I couldn't yet figure out what's the purpose of all the other XMLHttpRequest calls in the generated JS file because way I built mine didn't even require those?

This rises another question, why does emscripten build a lot of JS code it doesn't even use? 🤔

Ciantic avatar Jun 17 '22 22:06 Ciantic

I'm facing the same issue, which causes emscripten-compiled code not to be loadable in the background page of Chrome extensions using manifest V3.

This is how I solved it: implemented XMLHttpRequest using the fetch API, and load it before the emscripten-compiled code. This works well in my tests, and might be a good workaround to consider while emscripten fixes the issue at source.

class XMLHttpRequest {
  open(method, url) {
    if (method != 'GET') {
      console.error('Method not implemented:', method);
    }
    this._url = url;
  }

  get status() {
    return this._status;
  }
  

  get statusText() {
    return this._statusText;
  }

   set onprogress(callback) {
    // Fetch doesn't implement progress reporting, and it doesn't seem
    // emscripten actually needs it except for large files.
    this._onprogress = callback;
  }

  set onerror(callback) {
    this._onerror = callback;
  }

   set onload(callback) {
    this._onload = callback;
  }

  set responseType(value) {
    if (value != 'arraybuffer') {
      console.error('Response type not implemented', value);
    }
  }

  send() {
    fetch(this._url)
        .then(response => {
          this._status = response.status;
          this._statusText = response.statusText;
          return response.arrayBuffer();
        })
        .then((buffer) => {
          this._arrayBuffer = buffer;
          if (this._onload) {
            this._onload({});
          }
        })
        .catch((error) => {
          if (this._onerror) {
            console.error('Could not fetch:', error);
            this._onerror({});
          }
        });
  }

  get responseText() {
    console.error('Not implemented: getting responseText');
  }
  
  get response() {
    return this._arrayBuffer;
  }
}

// Import the emscripten-compiled module here.
importScripts('/my_fancy_module.js');

invernizzi avatar Sep 27 '22 13:09 invernizzi

bump

jimmywarting avatar May 20 '23 17:05 jimmywarting

I did look into this a while back and ran into the issue that we currently exports a synchronous fetch API, which Fetch does not support.. so we would either need to drop support for EMSCRIPTEN_FETCH_SYNCHRONOUS.. or we would need to include both backends in order to support it.

sbc100 avatar May 22 '23 23:05 sbc100

Sync xhr is kind of being removed from the main thread and service workers dont have xhr either.

So that leaves us with using jspi to suspend the assembly code with stack swimming

jimmywarting avatar May 23 '23 15:05 jimmywarting

We currently rely on sync xhr both on the main thread and on our workers / pthreads. If we want to move away from XMLhttpRequest, the first project would be to remove all of our dependencies on sync xhr. Without doing that we will just end up with two different fetch backends which will just increase code size.

sbc100 avatar May 23 '23 15:05 sbc100

Is someone working on this?

If not, how much of the current code depends on synchronous XHR?

Curve avatar May 27 '23 16:05 Curve

Usage of XHR in Emscripten's Fetch API currently makes it incompatible with Node.JS. The company I work for has a customer request for a Node-compatible build of our library, which we can't provide due to this issue. Is any work being done on this?

MAG-MichaelK avatar Nov 06 '23 10:11 MAG-MichaelK

Atm it looks like no one is working on this (this issue has no one assigned, and is still marked "good first bug" and "help welcome"). A PR would be very welcome! If someone wants to work on it but doesn't know where to start, let us know.

kripken avatar Nov 06 '23 21:11 kripken

maybe merged https://github.com/emscripten-core/emscripten/pull/22016 close this ?

pmp-p avatar Sep 27 '24 14:09 pmp-p

I think we still want to keep this open until the Emscripten Fetch API is updated.

We have a PR for that open, but it still has some work to go: https://github.com/emscripten-core/emscripten/pull/21199

sbc100 avatar Sep 27 '24 16:09 sbc100

I am currently implementing a work-around for this issue in my code base. It seems that the proposed solution #21199 has gone nowhere. I propose a minimal changeset implementing the existing API, not supporting synchronous fetches, using fetch() instead of XmlHttpRequest. This will be used by default for targets that do not support XmlHR like node.js or workers, and may optionally be chosen as the implementation for the web target similar to how you can pick sdl=1 or sdl=2. The current implementation would stay as it is for now. A possible future direction would be to extract the webbroser-only IndexDB into its own API, deprecate synchronous fetches and remove the fetch-implementation based on XmlHR

core-explorer avatar Jan 04 '25 18:01 core-explorer

I am currently implementing a work-around for this issue in my code base. It seems that the proposed solution #21199 has gone nowhere. I propose a minimal changeset implementing the existing API, not supporting synchronous fetches, using fetch() instead of XmlHttpRequest. This will be used by default for targets that do not support XmlHR like node.js or workers, and may optionally be chosen as the implementation for the web target similar to how you can pick sdl=1 or sdl=2. The current implementation would stay as it is for now. A possible future direction would be to extract the webbroser-only IndexDB into its own API, deprecate synchronous fetches and remove the fetch-implementation based on XmlHR

Have you looked at the wget API in emscripten/wget.h? It seems like that doesn't have any of the IndexDB or sync API issues so might be more suitable?

sbc100 avatar Jan 04 '25 18:01 sbc100

Thank you, I was not aware of the wget API, it seems to be missing from the official documentation. I found the header and I will see how it works for me

core-explorer avatar Jan 04 '25 19:01 core-explorer