Use Fetch in preference of XMLHttpRequest (on supported platforms)

[LPardue]

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.

[mikaelast]

Out of curiosity, has anything happened regarding this issue?

[stale]

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.

[Ciantic]

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.

denoland/deno#2191

[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

emscripten/src/web_or_worker_shell_read.js

Lines 48 to 68 in c0f1058

	readAsync = (url, onload, onerror) => {
	var xhr = new XMLHttpRequest();
	xhr.open('GET', url, true);
	xhr.responseType = 'arraybuffer';
	xhr.onload = () => {
	if (xhr.status == 200 || (xhr.status == 0 && xhr.response)) { // file URLs can return 0
	onload(xhr.response);
	return;
	}
	#if SUPPORT_BASE64_EMBEDDING
	var data = tryParseAsDataURI(url);
	if (data) {
	onload(data.buffer);
	return;
	}
	#endif
	onerror();
	};
	xhr.onerror = onerror;
	xhr.send(null);
	}

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? 🤔

[invernizzi]

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');

[jimmywarting]

bump

[sbc100]

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.

[jimmywarting]

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

[sbc100]

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.

[Curve]

Is someone working on this?

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

[MAG-MichaelK]

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?

[kripken]

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.

[pmp-p]

maybe merged #22016 close this ?

[sbc100]

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: #21199