rusackas opened a new pull request, #42811:
URL: https://github.com/apache/superset/pull/42811

   ### SUMMARY
   
   #42804 proposes reverting `simple-zstd` 1.4.2 → 2.1.0 (again) because `npm 
run dev-server` reportedly loses the connection and hangs after a few 
interactions (navigating between dashboards). This is a fix-forward instead of 
that revert.
   
   **Root cause:** `webpack.proxy-config.js`'s `processHTML()` pipes the 
proxied backend response through a decompression stream (`zlib` for 
gzip/br/deflate, `simple-zstd` for zstd) using plain `.pipe()`, then listens 
for `data`/`end`/`error` on the decompression stream to build the client 
response. `.pipe()` does **not** forward the source's errors or premature close 
to the destination — this is a well-known Node.js stream gotcha. So when the 
backend connection drops mid-response (most commonly: the Flask dev server's 
reloader restarting after a file save, which happens constantly during active 
backend+frontend co-development), the decompression stream is left waiting on 
input that will never arrive. Neither `end` nor `error` ever fires on it, 
`response.end()` is never called, and the request hangs indefinitely — 
eventually exhausting the browser's per-origin connection pool, which presents 
as "loses connection" and a stuck loading spinner.
   
   **This is not a `simple-zstd`-specific bug.** I reproduced the identical 
hang with `gzip` on the same (otherwise unmodified) code path — see the two 
parallel regression tests. It's a latent bug in `processHTML()`'s stream wiring 
that predates the `simple-zstd` v2 bump; it just started manifesting more once 
zstd-encoded responses became common (via `Flask-Compress` + `backports-zstd`, 
since zstd is typically preferred in content negotiation over gzip once 
available). Reverting the bump alone would not fix this — the same hang would 
keep happening on gzip-encoded pages.
   
   **Fix:** replace the manual `.pipe()` + event-listener wiring with 
`stream/promises`' `pipeline()`, which destroys every stream in the chain and 
rejects as soon as any one of them errors or closes prematurely. The existing 
`onProxyRes` `.catch()` handler already does the right thing with that 
rejection (logs it, sends a clear error response) — it just never used to 
receive it.
   
   Also added `tools/webpack.proxy-config.test.js`; there was no test coverage 
at all for this file previously.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A — dev-tooling fix, no UI change.
   
   ### TESTING INSTRUCTIONS
   
   ```bash
   cd superset-frontend
   npx jest tools/webpack.proxy-config.test.js
   ```
   
   Three tests:
   1. A complete zstd-encoded HTML response still decompresses correctly (no 
regression).
   2. A backend connection that drops mid-response while streaming zstd-encoded 
HTML now fails fast with a clear error instead of hanging.
   3. Same as (2), but with gzip, proving the bug (and fix) aren't 
zstd-specific.
   
   Confirmed (2) and (3) **hang indefinitely** (whole test process needs to be 
killed) against the pre-fix code, and pass in a few seconds with the fix 
applied.
   
   Manual verification: `npm run dev-server` against a real backend, 
killing/restarting the Flask process mid-navigation repeatedly — the in-flight 
page request now fails with a visible proxy error instead of spinning forever, 
and subsequent navigations are unaffected.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   Related: #42804 (proposed revert), #38662 / #39138 / #39139 / #39369 (prior 
`simple-zstd` bump/revert history)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to