A dead or erroring Web Worker leaves the client permanently wedged
- 主要言語
- TypeScript
- スター
- 1
- フォーク
- 21
- 平均マージ
- 12時間 14分
- マージ済み PR(30日)
- 41
説明
Found while reviewing #313, which routes batch submission through the worker and so made this path matter more. Split out because the fix is not a drive-by: it changes shared client lifecycle for every consumer, and a partial version is worse than none.
## The bug
`WebClient` registers only a `"message"` listener on its worker (`crates/web-client/js/index.js`, in the constructor). There is no `"error"` or `"messageerror"` handler anywhere in the file. So:
- **Worker script fails to load or parse.** The spec fires `error` at the `Worker` and nothing else. `this.ready` never settles, so `createClient()` awaits forever. Given the SDK's worker does `await import("./Cargo-*.js")`, a bundler misconfiguration lands exactly here — and the symptom is a hang with no error, which is about the worst possible diagnostic.
- **Uncaught error in the worker.** `error` fires; nothing settles the in-flight request. Because `_serializeWasmCall` chains on the pending promise, that one never-settling request blocks every later call behind it. One dead call takes the whole client with it.
Measured by driving the shipped `js/index.js` against a fake worker: the in-flight call never settles, and a subsequent `waitForIdle()` and an unrelated `newAccount()` hang forever too.
## Why the obvious fix is not enough
Rejecting the entries in `pendingRequests` on `error` releases the current cohort and the chain slot — and then the very next call re-wedges the client. `ready` has already resolved, so rejecting it is a no-op; `callMethodWithWorker` sails past `await this.ready`, inserts a fresh id into the just-cleared map, and posts to the corpse. Measured: the second call hangs, and so does anything queued behind it.
A correct fix needs a terminal state, not just a flush:
- latch a `workerFailure` on the instance and have `callMethodWithWorker` reject immediately when it is set, so later calls fail fast with a clear error instead of hanging. `assertNotTerminated` in `client.js` is the existing precedent for this shape.
- decide what to do about late responses. An uncaught runtime error does **not** terminate a worker — per the HTML Standard the UA reports it and fires `error`, but the worker's event loop continues. So the request may still complete. Today a late success is silently dropped, and a late *error* response is not even logged (`pendingRequests.has(requestId)` is false, and the init-failure branch requires `!requestId`). Failing all callers on a non-fatal error can also convert a healthy batch into a reported failure.
- `event.message` is absent for a load failure (a plain `Event`) and can be `""`, so `??` alone gives `"worker failed — "`. The two cases want different messages anyway.
- consider `preventDefault()`: the worker `error` event is cancelable, and not cancelling it means the UA re-reports the exception on the parent global on top of whatever we log.
- weigh the unhandled-rejection change. Rejecting promises nobody awaits turns a silent hang into an `unhandledrejection`, which is an improvement in most apps but new noise on a page with a strict reporter.
- `messageerror` has limited browser support and this SDK deliberately routes Safari/WKWebView to a classic worker, so that half would not fire there.
## Note on scope
Not caused by #313, and not batching-specific — it is the worker lifecycle for all ~40 forwarded methods. Worth fixing with a test: aliasing the `../Cargo.toml` import to a stub makes `js/index.js` importable under vitest with a fake `Worker`, which is how the behaviour above was measured. That would also close the standing gap that `js/index.js` has no unit tests at all.
コントリビューションガイド
調査の方向性
The bug is in crates/web-client/js/index.js, where the WebClient constructor lacks error handlers for the Worker. Start by examining the constructor and the callMethodWithWorker method. Set up a test environment using vitest with a fake Worker to reproduce the hang. The fix involves adding error and messageerror listeners, managing a terminal failure state, and handling late responses. Look at assertNotTerminated in client.js for precedent on latching a failure state.
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- javascript, typescript, webpack
- 領域
- frontend, testing-qa, web-dev
- issue の種類
- バグ
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 活発さ
- 活発
- 明瞭さ
- 明確に書かれている
- 初心者へのやさしさ
- 45/100