0xMiden / 0xMiden/web-sdk

A dead or erroring Web Worker leaves the client permanently wedged

オープン
#315 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
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

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。