`-sFETCH_STREAMING`: a failed request crashes if `onerror` calls `emscripten_fetch_close`, and otherwise gets two callbacks

[RobinTF]

With -sFETCH_STREAMING, when a streaming request (EMSCRIPTEN_FETCH_STREAM_DATA) fails (connection refused, connection dropped mid-body, or timeout):

• If onerror calls emscripten_fetch_close, as the docs example does, onerror recurses until the program crashes.
• Otherwise, a second callback follows: onerror again, or onsuccess if the status was 2xx. So a truncated download is reported as a success.

Guarding the close with fetch->status != (uint16_t)-1, as test_fetch_stream_abort.cpp does, avoids the crash, but onerror is still called a second time, with status -1 ("aborted with emscripten_fetch_close()").

This is different from #21005: there the fetch was still running when it was closed, whereas here the request has already failed, and on the XHR path onerror is only called once the fetch is DONE, so closing in it is safe.

The docs promise "either the success or the failure callback".

Reproduction (Emscripten 6.0.10, Node.js)

// probe.c
#include <emscripten/fetch.h>
#include <stdio.h>
#include <string.h>

static void onsuccess(emscripten_fetch_t* f) { printf("onsuccess\n"); }
static void onerror(emscripten_fetch_t* f) {
  printf("onerror\n");
#ifdef CLOSE
  emscripten_fetch_close(f);
#endif
}

int main(int argc, char** argv) {
  emscripten_fetch_attr_t attr;
  emscripten_fetch_attr_init(&attr);
  strcpy(attr.requestMethod, "GET");
  attr.attributes = EMSCRIPTEN_FETCH_LOAD_TO_MEMORY | EMSCRIPTEN_FETCH_STREAM_DATA;
  attr.onsuccess = onsuccess;
  attr.onerror = onerror;
  emscripten_fetch(&attr, argv[1]);
}

// server.js: send part of the body, then drop the connection.
require("http").createServer((req, res) => {
  res.writeHead(200, {"Content-Length": "100000"});
  res.write("x".repeat(1000));
  setTimeout(() => res.socket.destroy(), 100);
}).listen(8767);

$ node server.js &
$ emcc probe.c -O1 -sFETCH -sFETCH_STREAMING -o probe.js && node probe.js http://127.0.0.1:8767/
onerror
onsuccess
$ emcc probe.c -O1 -DCLOSE -sFETCH -sFETCH_STREAMING -o probe.js && node probe.js http://127.0.0.1:8767/
onerror
onerror
... (about 8000 more)
RangeError: Maximum call stack size exceeded

Cause

In FetchXHR.send() (src/Fetch.js):

1. The catch block calls onerror/ontimeout before readyState is 4 (DONE). emscripten_fetch_close treats the fetch as still in flight and calls onerror again to report the cancellation, which closes again, and so on.
2. The finally block calls onload even after a failure, so fetchXHR reports the request a second time.

A real XMLHttpRequest sets readyState to 4 first, and then fires exactly one of load, error or timeout.

Possible fix

Fire exactly one event, after the state is DONE. This fixes both cases above, and a successful request still reports onsuccess.

--- a/src/Fetch.js
+++ b/src/Fetch.js
@@ -156,6 +156,7 @@ class FetchXHR {
       credentials: this.withCredentials ? 'include' : 'same-origin',
     };
 
+    let failure = null;
     try {
       const response = await fetch(this._url, fetchOptions);
 
@@ -218,23 +219,21 @@ class FetchXHR {
         this.response = allChunks.buffer;
       }
     } catch (error) {
+      failure = error;
       this.statusText = error.message;
-
-      if (error.name === 'AbortError') {
-        // Do nothing.
-      } else if (error.name === 'TimeoutError') {
-        this.ontimeout?.();
-      } else {
-        // This is a network error
-        this.onerror?.();
-      }
     } finally {
       clearTimeout(timeoutID);
       if (!this._aborted) {
         this._changeReadyState(4); // 4: DONE
         // The XHR 'load' event fires for successful HTTP statuses (2xx) as well as
         // unsuccessful ones (4xx, 5xx). The 'error' event is for network failures.
-        this.onload?.();
+        if (!failure) {
+          this.onload?.();
+        } else if (failure.name === 'TimeoutError') {
+          this.ontimeout?.();
+        } else if (failure.name !== 'AbortError') {
+          this.onerror?.();
+        }
       }
     }
   }

[RobinTF]

To give some context on this issue:
I actually used AI to learn a bit more about this codebase and I wanted to know the limitations of the fetch API (which I found to be a bit disappointing, so I had to implement my own fetch() wrapper, which was a huge pain because of the async).
During this process the AI told me that this bug exists. I didn't stumble upon it in the wild, but it seems to be there, so this issue is me just letting you know about this. I don't use this API for anything, so you can safely consider this low priority. Have a nice day!