mirror of
https://github.com/PR0M3TH3AN/bitlogin.git
synced 2026-10-27 08:31:21 +00:00
widget: make an unmounted-widget teardown loud, not a silent hang
Removing <bitlogin-auth> from the DOM terminates its crypto worker. A host that re-renders surrounding markup (framework diff, htmx swap, innerHTML reset) can do this incidentally, after which signing calls fail -- and the failure was hard to diagnose. Two guardrails: - disconnectedCallback logs a console.error when the element is removed while a session (BitLogin account or NIP-07/46) is active, naming the unmount as the cause and pointing at the README "Integration gotcha". - WorkerClient.terminate(reason) records the reason; both in-flight calls (via failAll) and any later call (via the dead-flag path) reject with it, so an unmounted widget produces one legible error message instead of a bare or mismatched one. The dead flag already prevented the infinite hang; this makes the cause visible and unifies the wording. Adds worker/workerClient.test.ts (fake Worker) covering both rejection paths. Updates the README gotcha paragraphs from "hangs forever with no error" to the new loud-rejection behavior. Addresses the "unmount guardrail" item of the Tessera integration feedback (docs/TODO.md). Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01BXQCvJPA6sVSQ5j8uBd9bB
This commit is contained in:
1 parent
c9791b9840
commit
86d7ee0114
5 files changed
+111
-15
No files matched your search
@@ -168,9 +168,13 @@ crypto Web Worker (that's the correct cleanup when the element is genuinely
|
||||
being removed). If your host app re-renders its own DOM by resetting an
|
||||
ancestor's `innerHTML` — a common pattern in hand-rolled UI without a
|
||||
framework's reconciler — and the widget happens to be a descendant of that
|
||||
subtree, this **silently kills the in-progress session**: `getPublicKey()`
|
||||
subtree, this **kills the in-progress session**: `getPublicKey()` that
|
||||
already resolved before the render still looks fine, but every call after
|
||||
that hangs forever, because the worker is gone and nothing ever replies.
|
||||
that fails. As of the unmount guardrail, that failure is now loud rather
|
||||
than silent — the element logs a `console.error` on disconnect-while-active,
|
||||
and every later `signEvent`/`getPublicKey` **rejects** with a message naming
|
||||
the unmount as the cause, instead of hanging forever. It's still a broken
|
||||
session, though; the fix below is unchanged.
|
||||
Mount `<bitlogin-auth>` into a container that your own re-renders never
|
||||
replace (toggle visibility with the `hidden` attribute instead of removing
|
||||
it from the DOM), not into a template string that gets thrown away and
|
||||
@@ -191,8 +195,12 @@ a genuine remove-and-recreate. The worker still gets terminated, and
|
||||
because `connectedCallback` doesn't construct a new `WorkerClient` (that
|
||||
only happens once, in the constructor), the element is left reusing a
|
||||
now-dead worker: every subsequent message — `configure`, `login`,
|
||||
everything — silently hangs forever with no error, since `postMessage` on
|
||||
a terminated `Worker` is a no-op. BitUnlock hit exactly this while trying
|
||||
everything — fails, since `postMessage` on a terminated `Worker` is a
|
||||
no-op. The client now marks itself torn-down on `terminate()`, so those
|
||||
calls **reject immediately** with a clear teardown message (and the removal
|
||||
logged a `console.error`) rather than hanging forever with no error — but
|
||||
the element is still stuck on a dead worker, so this is a diagnosis, not a
|
||||
recovery. BitUnlock hit exactly this while trying
|
||||
to work around the innerHTML gotcha above: it kept `#bitloginMount` static
|
||||
but used `replaceWith()` to relocate it into a per-step placeholder,
|
||||
believing a move (vs. removal) would spare the worker. It doesn't. The fix
|
||||
|
||||
+11
-6
@@ -386,12 +386,17 @@ how much they cost.
|
||||
|
||||
### Medium
|
||||
|
||||
- **The unmount-kills-the-worker footfooter deserves a guardrail.** The docs
|
||||
warn that removing `<bitlogin-auth>` tears down the crypto worker and hangs
|
||||
every later signing call with no error. I hit exactly this. Consider: (a) a
|
||||
console.error when the element is disconnected while a session is active, and
|
||||
(b) `signEvent` rejecting with a clear "widget was unmounted" error instead of
|
||||
hanging forever. Silent infinite-hang is the worst failure mode.
|
||||
- **[DONE 2026-08-11] The unmount-kills-the-worker footgun now has a guardrail.**
|
||||
The docs warned that removing `<bitlogin-auth>` tears down the crypto worker
|
||||
and hangs every later signing call with no error. **Shipped both asks:**
|
||||
(a) `disconnectedCallback` now logs a `console.error` when the element is
|
||||
removed while a session is active, naming the unmount and pointing at the
|
||||
README "Integration gotcha"; and (b) `WorkerClient.terminate(reason)` records
|
||||
the reason and `signEvent`/`getPublicKey`/etc. — whether in-flight at teardown
|
||||
or issued afterward — reject with that message instead of hanging (the
|
||||
`dead`-flag path already prevented the infinite hang; this makes the *cause*
|
||||
legible and unifies the message). Covered by `worker/workerClient.test.ts`;
|
||||
README gotcha paragraphs updated to say "rejects loudly," not "hangs forever."
|
||||
- **Styling hooks / CSS parts.** The widget renders in a shadow root, so a host
|
||||
can only frame it, not theme it. A few `::part()` hooks (button, panel, text)
|
||||
would let it match a host design system instead of looking pasted in. Tessera
|
||||
|
||||
@@ -238,9 +238,30 @@ export class BitLoginAuthElement extends HTMLElement {
|
||||
|
||||
disconnectedCallback(): void {
|
||||
if (this.successDismissTimer !== null) clearTimeout(this.successDismissTimer);
|
||||
// Captured before clearActiveSession() below wipes the signal.
|
||||
const hadActiveSession = Boolean(this.session || this.altSigner);
|
||||
clearActiveSession(this);
|
||||
this.releaseSigner();
|
||||
this.worker.terminate();
|
||||
// Removing <bitlogin-auth> from the DOM tears down its crypto worker. That's
|
||||
// correct on a genuine teardown, but a host that re-renders its surrounding
|
||||
// markup (a framework diff, an htmx swap, an innerHTML assignment) can remove
|
||||
// and re-insert the element incidentally -- and then every later
|
||||
// signEvent/getPublicKey call rejects, which is baffling without the cause.
|
||||
// Say so loudly rather than letting the app hit silent rejections. The fix is
|
||||
// to mount the element into a container the host's re-renders never replace
|
||||
// (README "Integration gotcha").
|
||||
if (hadActiveSession) {
|
||||
console.error(
|
||||
"[BitLogin] <bitlogin-auth> was removed from the DOM while a session was active. " +
|
||||
"Its crypto worker is now torn down, so later signEvent/getPublicKey calls will reject. " +
|
||||
"If this was an incidental re-render, mount the element in a stable container your re-renders " +
|
||||
"don't replace; see the BitLogin README, \"Integration gotcha\"."
|
||||
);
|
||||
}
|
||||
this.worker.terminate(
|
||||
"BitLogin's <bitlogin-auth> element was removed from the DOM, tearing down its signer. " +
|
||||
"Re-mount the widget or reload the page to sign in again."
|
||||
);
|
||||
}
|
||||
|
||||
// ---- Public API mirroring window.nostr, scoped to this element instance ----
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { WorkerClient } from "./workerClient.js";
|
||||
|
||||
/** Minimal stand-in for a real Worker: records posted messages but never
|
||||
* answers, so a call stays pending until the client tears it down -- exactly
|
||||
* the state an unmounted <bitlogin-auth> leaves behind. */
|
||||
class FakeWorker {
|
||||
static instances: FakeWorker[] = [];
|
||||
listeners: Record<string, ((event: unknown) => void)[]> = {};
|
||||
posted: unknown[] = [];
|
||||
terminated = false;
|
||||
constructor(_url: URL, _opts?: unknown) {
|
||||
FakeWorker.instances.push(this);
|
||||
}
|
||||
addEventListener(type: string, cb: (event: unknown) => void): void {
|
||||
(this.listeners[type] ??= []).push(cb);
|
||||
}
|
||||
postMessage(msg: unknown): void {
|
||||
this.posted.push(msg);
|
||||
}
|
||||
terminate(): void {
|
||||
this.terminated = true;
|
||||
}
|
||||
}
|
||||
|
||||
describe("WorkerClient teardown", () => {
|
||||
beforeEach(() => {
|
||||
FakeWorker.instances = [];
|
||||
(globalThis as unknown as { Worker: unknown }).Worker = FakeWorker;
|
||||
});
|
||||
afterEach(() => {
|
||||
delete (globalThis as unknown as { Worker?: unknown }).Worker;
|
||||
});
|
||||
|
||||
it("rejects an in-flight call with the teardown reason instead of hanging", async () => {
|
||||
const client = new WorkerClient();
|
||||
const pending = client.getPublicKey();
|
||||
client.terminate("widget was unmounted");
|
||||
await expect(pending).rejects.toThrow("widget was unmounted");
|
||||
expect(FakeWorker.instances[0]?.terminated).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects calls made after teardown immediately, with the same reason", async () => {
|
||||
const client = new WorkerClient();
|
||||
client.terminate("widget was unmounted");
|
||||
await expect(client.getPublicKey()).rejects.toThrow("widget was unmounted");
|
||||
});
|
||||
|
||||
it("falls back to a clear default reason when terminate() is given none", async () => {
|
||||
const client = new WorkerClient();
|
||||
const pending = client.signEvent({ kind: 1, content: "hi", tags: [] } as never);
|
||||
client.terminate();
|
||||
await expect(pending).rejects.toThrow(/torn down|reload the page/i);
|
||||
});
|
||||
});
|
||||
@@ -53,6 +53,10 @@ export class WorkerClient {
|
||||
>();
|
||||
private counter = 0;
|
||||
private dead = false;
|
||||
/** Why the worker is gone, surfaced on every post-teardown rejection so a
|
||||
* developer sees the CAUSE (usually an unmounted element), not just a bare
|
||||
* failure. Overridable via terminate(reason). */
|
||||
private deadReason = "This BitLogin session was torn down; reload the page to sign in again.";
|
||||
/** Set by the element to receive unsolicited worker frames (auth_url). */
|
||||
onNotification: ((notification: WorkerNotification) => void) | null = null;
|
||||
|
||||
@@ -120,7 +124,7 @@ export class WorkerClient {
|
||||
timeoutMs = CALL_TIMEOUT_MS
|
||||
): Promise<WorkerActionMap[A][1]> {
|
||||
if (this.dead) {
|
||||
return Promise.reject(new Error("This BitLogin session was torn down; reload the page to sign in again."));
|
||||
return Promise.reject(new Error(this.deadReason));
|
||||
}
|
||||
const id = `${Date.now().toString(36)}-${(this.counter++).toString(36)}`;
|
||||
return new Promise((resolve, reject) => {
|
||||
@@ -248,11 +252,14 @@ export class WorkerClient {
|
||||
return this.call("logout", {});
|
||||
}
|
||||
|
||||
terminate(): void {
|
||||
terminate(reason?: string): void {
|
||||
this.dead = true;
|
||||
if (reason) this.deadReason = reason;
|
||||
this.worker.terminate();
|
||||
// terminate() silently kills in-flight work; without this every caller
|
||||
// waits forever on a worker that no longer exists.
|
||||
this.failAll("The BitLogin crypto worker was terminated.");
|
||||
// waits forever on a worker that no longer exists. Reject with the same
|
||||
// reason later calls get, so an in-flight signEvent and a subsequent one
|
||||
// fail identically and legibly.
|
||||
this.failAll(this.deadReason);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user