Rewrite shareurl race more declaratively (Promise.any + side-race)

Joe says:

I think I constrained Claude so much in this session that it had to say things
I like below :-)

This is post-convo with Ben where we were still unhappy with (a) variable names
and (b) closed over mutable variables instead of declarative promise APIs.

Claude says:

Following a second pass of code review: the previous version used a
new Promise((resolve, reject) => { let gotSomeResponse = false; ... })
block with a flag tracking which side had won. That conflated two
concerns — "first success wins the response" and "abort proxy iff direct
beat it" — into one imperative state machine.

This pass separates them. The race is expressed as three composed
Promise primitives:

  - directP: direct fetch, gated on content-type verification (rejects
    if direct returned the wrong shape, so it drops out of the response
    race entirely).
  - shouldProxyPromise (Promise.race): false iff direct verified before
    the timeout; locks the per-host policy for the page-load.
  - directFinishedSuccessfullyAndFirstP (Promise.race): does direct
    settle before proxy, AND was that settlement a verified success?
    Only then do we proxyCtrl.abort(). This keeps the abort safe — we
    never abort once proxy has already returned headers, which would
    error the caller's body stream mid-read.
  - responsePromise (Promise.any): whichever of directP or proxyP
    fulfills first.

Tested both healthy and DevTools-blocked flows. Observed the timing
edge case where both proxy and direct return 200 within a few ms of
each other: shouldProxy still locks 'direct' as expected, since the
verdict is decoupled from whether the abort caught proxy in time.

Also adds a comment explaining the deliberate non-handling of caller-
provided init.signal: the signal overwrite means a caller aborting
won't cancel the proxy fetch, but the alternative (event-listener
forwarding or AbortSignal.any) is more complexity than the
not-quite-fully-aborted case justifies right now.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: jpolitz

src/web/js/beforePyret.js

@@ -59,36 +59,45 @@ function _shareurlFetch(shouldProxy, fetchInput, fetchInit) {
 
 function _shareurlRace(fetchInput, fetchInit) {
   const proxyCtrl = new AbortController();
+  // NOTE(joe): The signal overwrite is technically not the right fetch()
+  // polyfill. If the caller elsewhere in the codebase provided a different
+  // signal (which in the fetch API is only for aborting as of April '26), that
+  // caller aborting through that signal won't cancel the proxy fetch.
+  // I'm OK letting that case slip through here in exchange for not having a
+  // bunch of extra event handler forwarding
   const proxyP = _origFetch(_shareurlProxyUrl(fetchInput),
     Object.assign({}, fetchInit, { signal: proxyCtrl.signal }));
-  const directP = _origFetch(fetchInput, fetchInit);
+  const directP = _origFetch(fetchInput, fetchInit).then(r => {
+    if (!_shareurlVerifyDirect(r)) throw new Error('direct request failed');
+    return r;
+  });
 
-  // shouldProxy is decided from direct's headers — a timeout flips us to
-  // true if direct hangs at the network level (no headers, no error).
+  // shouldProxy: false iff direct verified before the timeout, else true.
+  // Whether to proxy is decided solely on whether direct succeeds or not
   const shouldProxyPromise = Promise.race([
-    directP.then(r => !_shareurlVerifyDirect(r), () => true),
+    directP.then(() => false, () => true),
     new Promise(resolve => setTimeout(() => resolve(true), SHAREURL_DIRECT_TIMEOUT_MS)),
   ]);
 
-  // Caller's response: direct if its headers verify (and abort the proxy
-  // fetch to stop wasting server bandwidth); otherwise proxy. If proxy also
-  // fails, surface its error.
-  const responsePromise = new Promise((resolve, reject) => {
-    let gotSomeResponse = false;
-    directP.then(r => {
-      if (!gotSomeResponse && _shareurlVerifyDirect(r)) {
-        gotSomeResponse = true;
-        proxyCtrl.abort();
-        resolve(r);
-      }
-    }).catch(() => { /* wait for proxy to resolve or reject */ });
-    proxyP.then(r => {
-      if (!gotSomeResponse) { gotSomeResponse = true; resolve(r); }
-    }).catch(e => {
-      if (!gotSomeResponse) reject(e);
-    });
+  // Settlement-order check: if direct verifies before proxy returns, abort
+  // the in-flight proxy to stop wasting server bandwidth. We must NOT
+  // abort once proxy has already returned, since by then the caller is
+  // reading proxy's body and aborting would error its stream mid-read.
+  const directFinishedSuccessfullyAndFirstP = Promise.race([
+    directP.then(() => true, () => false),
+    proxyP.then(() => false, () => false),
+  ]);
+  directFinishedSuccessfullyAndFirstP.then(directFirst => {
+    if (directFirst) proxyCtrl.abort();
   });
 
+  // Caller's response: whichever of direct-verified or proxy fulfills
+  // first. If both fail, surface proxy's error (the more authoritative
+  // upstream — direct's may just be 'direct-not-verified').
+  const responsePromise = Promise.any([directP, proxyP]).catch(
+    aggErr => Promise.reject(aggErr.errors[1] || aggErr.errors[0])
+  );
+
   return { responsePromise, shouldProxyPromise };
 }