Worker callback bridge: non-Error throws are silently treated as success, and the 30s ceiling is not configurable

[WiktorStarczewski]

Two defects in the worker's external-keystore callback bridge. Both are pre-existing and affect every worker-forwarded method, not one call site. Splitting them out of #313, which made batching a forwarded method and so widened the set of affected calls.

1. A throw whose value has no truthy .message loses its reason

The main-thread bridge serializes a failed callback as error.message:

// crates/web-client/js/index.js — EXECUTE_CALLBACK handler
} catch (error) {
  this.worker.postMessage({
    callbackError: error.message,
    callbackRequestId: requestId,
  });
}

The worker then treats a falsy callbackError as success:

// crates/web-client/js/workers/web-client-methods-worker.js — self.onmessage
if (!callbackError) {
  resolve(callbackResult);
} else {
  reject(new Error(callbackError));
}

A callback that throws a value with no truthy .message — throw "user rejected", or a wallet-style throw { code: 4001 }, or an Error with an empty message — yields callbackError === undefined, and the worker resolves with undefined. A thrown object that does carry a message (including a plain { message: "rejected" }) propagates correctly; a thrown null/undefined is worse still, since reading .message throws inside the handler, no response is posted at all, and the call hangs until the 30s timeout in defect 2.

The signature does not then succeed: web_keystore_callbacks.rs rejects the resolved value because it is not a Uint8Array, so the call fails with

sign callback must return a Uint8Array

That is the whole defect — not a forged approval, but a misclassification that destroys the real reason. The user rejected; the developer sees a return-type complaint that points at their callback's signature instead of at the rejection, with no code, no name, no cause. For a wallet integration that is the difference between "user declined" and an apparent SDK bug.

Fix: send an explicit success/failure discriminator instead of inferring failure from the truthiness of a message string, and serialize enough of the thrown value to preserve code / name / cause.

2. The 30-second per-callback ceiling is fixed

const CALLBACK_TIMEOUT_MS = 30000;

Applied to getKey, insertKey, and sign. A signature that needs physical confirmation — a hardware wallet, a lock-aware keystore, any human-in-the-loop approval — can exceed 30 seconds, and the call fails with Callback <id> timed out.

Batching sharpens this. Signing happens inside BatchBuilder::push, and this wrapper treats a failed push as fatal, so submit() is never reached: one slow approval fails the whole batch, where ten single submits would have lost only one.

Fix: make the ceiling configurable (a ClientOptions field), and consider no timeout by default for sign, where the wait is expected to be human-paced.

Why not fixed in #313

Both defects are in the shared bridge used by all worker-forwarded methods. A fix scoped to batching would leave the other ~40 methods broken while adding a third execution path to reason about — the same reasoning that split #315 out.

Related

• perf: run transaction batches in the worker instead of the main thread #313 — made batching a forwarded method
• A dead or erroring Web Worker leaves the client permanently wedged #315 — worker lifecycle: a dead worker wedges the client

[kutluhaneth46]

Taking this next (after #320). Plan: explicit success/failure discriminator on the worker callback bridge (stop inferring failure from truthy error.message), preserve code/name/message from thrown values, and make the callback timeout configurable via client options (no fixed 30s ceiling for human-paced sign).