fix(ext/popup): defensive Promise.allSettled in devices (Plan C Phase 5)
DEV-C P2: Promise.all meant one rejected RPC failed the whole render.
allSettled + per-slot fallback keeps the active-devices surface usable
when the revoked-list feed (or one bad ssh fingerprint) is down.
Two call sites converted in devices.ts:
1. list_devices + list_revoked pair — revoked failures now render an
inline "couldn't load" slot instead of failing the page.
2. sshFingerprint map — one bad public key falls back to '(unknown)'
instead of killing the whole device list.
trash.ts only has a single sendMessage in its load path on this branch,
so it has no Promise.all to migrate. Plan was written against a slightly
different snapshot; documented divergence in report.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -95,6 +95,32 @@ describe('devices view', () => {
|
||||
expect(app.querySelector<HTMLButtonElement>('#register-confirm-btn')).not.toBeNull();
|
||||
});
|
||||
|
||||
// Plan C Phase 5 — defensive Promise.allSettled:
|
||||
// a rejected secondary feed (list_revoked) should not kill the whole render.
|
||||
it('renders devices when revoked list fails (load-error slot shown)', async () => {
|
||||
(sendMessage as ReturnType<typeof vi.fn>)
|
||||
.mockResolvedValueOnce({ ok: true, data: { devices: [{ name: 'CLI', public_key: 'k', added_at: 1 }] } })
|
||||
.mockRejectedValueOnce(new Error('boom'));
|
||||
|
||||
await renderDevices(app);
|
||||
|
||||
// Primary list still rendered.
|
||||
expect(app.innerHTML).toContain('CLI');
|
||||
// Inline fallback slot present.
|
||||
expect(app.innerHTML).toContain("Couldn't load revoked devices");
|
||||
});
|
||||
|
||||
it('renders devices when revoked list returns {ok:false}', async () => {
|
||||
(sendMessage as ReturnType<typeof vi.fn>)
|
||||
.mockResolvedValueOnce({ ok: true, data: { devices: [{ name: 'CLI', public_key: 'k', added_at: 1 }] } })
|
||||
.mockResolvedValueOnce({ ok: false, error: 'list_revoked_failed' });
|
||||
|
||||
await renderDevices(app);
|
||||
|
||||
expect(app.innerHTML).toContain('CLI');
|
||||
expect(app.innerHTML).toContain("Couldn't load revoked devices");
|
||||
});
|
||||
|
||||
it('confirming register sends register_this_device with the entered name', async () => {
|
||||
(chrome.storage.local.get as ReturnType<typeof vi.fn>).mockResolvedValueOnce({ device_name: 'Unknown' });
|
||||
// Initial render: list_devices + list_revoked.
|
||||
|
||||
@@ -31,35 +31,64 @@ export function teardown(): void {
|
||||
// No cleanup needed
|
||||
}
|
||||
|
||||
/**
|
||||
* DEV-C P2: defensive per-slot rendering. The active list is the primary
|
||||
* feed — if it fails entirely, we still surface an error page. The
|
||||
* revoked list is secondary — its failure renders an inline "couldn't
|
||||
* load" slot but doesn't kill the page.
|
||||
*/
|
||||
function revokedLoadErrorHtml(): string {
|
||||
return `
|
||||
<details class="revoked-section">
|
||||
<summary class="muted">▸ revoked devices</summary>
|
||||
<div class="revoked-section__body">
|
||||
<p class="muted">Couldn't load revoked devices.</p>
|
||||
</div>
|
||||
</details>
|
||||
`;
|
||||
}
|
||||
|
||||
export async function renderDevices(app: HTMLElement): Promise<void> {
|
||||
// Get current device name from local storage
|
||||
const stored = await chrome.storage.local.get(['device_name']);
|
||||
const currentDeviceName: string | undefined = stored.device_name as string | undefined;
|
||||
|
||||
// Fetch active device list and revoked list in parallel
|
||||
const [devicesResp, revokedResp] = await Promise.all([
|
||||
// Fetch active device list and revoked list in parallel. allSettled so a
|
||||
// rejected secondary feed doesn't kill the whole render.
|
||||
const [devicesSettled, revokedSettled] = await Promise.allSettled([
|
||||
sendMessage({ type: 'list_devices' }),
|
||||
sendMessage({ type: 'list_revoked' }),
|
||||
]);
|
||||
|
||||
if (!devicesResp.ok) {
|
||||
if (devicesSettled.status === 'rejected' || !devicesSettled.value.ok) {
|
||||
app.innerHTML = `<div class="pad"><p class="error">Failed to load devices</p></div>`;
|
||||
return;
|
||||
}
|
||||
|
||||
const devices = (devicesResp.data as { devices: Device[] }).devices;
|
||||
const revokedDevices: RevokedEntry[] = revokedResp.ok
|
||||
? (revokedResp.data as { revoked: RevokedEntry[] }).revoked
|
||||
// devicesSettled.value.ok is true here (guarded above), so .data is present.
|
||||
const devicesData = (devicesSettled.value as { ok: true; data: unknown }).data;
|
||||
const devices = (devicesData as { devices: Device[] }).devices;
|
||||
const revokedOk = revokedSettled.status === 'fulfilled' && revokedSettled.value.ok;
|
||||
const revokedDevices: RevokedEntry[] = revokedOk
|
||||
? ((revokedSettled.value as { ok: true; data: unknown }).data as { revoked: RevokedEntry[] }).revoked
|
||||
: [];
|
||||
|
||||
const isRegistered = currentDeviceName && devices.some((d) => d.name === currentDeviceName);
|
||||
|
||||
// Precompute fingerprints for all active devices
|
||||
// Precompute fingerprints for all active devices. allSettled so one bad
|
||||
// public key doesn't kill the whole list — fall back to '(unknown)'.
|
||||
const fingerprints = new Map<string, string>();
|
||||
await Promise.all(devices.map(async (d) => {
|
||||
const fp = await sshFingerprint(d.public_key);
|
||||
fingerprints.set(d.name, fp ?? '(unknown)');
|
||||
}));
|
||||
const fpResults = await Promise.allSettled(
|
||||
devices.map((d) => sshFingerprint(d.public_key).then((fp) => [d.name, fp] as const)),
|
||||
);
|
||||
for (let i = 0; i < devices.length; i += 1) {
|
||||
const r = fpResults[i];
|
||||
if (r.status === 'fulfilled' && r.value[1]) {
|
||||
fingerprints.set(r.value[0], r.value[1]);
|
||||
} else {
|
||||
fingerprints.set(devices[i].name, '(unknown)');
|
||||
}
|
||||
}
|
||||
|
||||
const activeDevicesHtml = devices.length === 0
|
||||
? `<p class="muted" style="text-align:center;margin-top:32px;">No devices registered</p>`
|
||||
@@ -82,7 +111,9 @@ export async function renderDevices(app: HTMLElement): Promise<void> {
|
||||
`;
|
||||
}).join('');
|
||||
|
||||
const revokedSectionHtml = revokedDevices.length === 0 ? '' : `
|
||||
const revokedSectionHtml = !revokedOk
|
||||
? revokedLoadErrorHtml()
|
||||
: revokedDevices.length === 0 ? '' : `
|
||||
<details class="revoked-section">
|
||||
<summary class="muted">▸ show ${revokedDevices.length} revoked device${revokedDevices.length !== 1 ? 's' : ''}</summary>
|
||||
<div class="revoked-section__body">
|
||||
@@ -117,7 +148,7 @@ export async function renderDevices(app: HTMLElement): Promise<void> {
|
||||
` : ''}
|
||||
${devices.length > 0 ? `<div class="section-header">ACTIVE · ${devices.length}</div>` : ''}
|
||||
${activeDevicesHtml}
|
||||
${revokedDevices.length > 0 ? `<div class="section-header">REVOKED · ${revokedDevices.length}</div>` : ''}
|
||||
${!revokedOk ? `<div class="section-header">REVOKED · ?</div>` : (revokedDevices.length > 0 ? `<div class="section-header">REVOKED · ${revokedDevices.length}</div>` : '')}
|
||||
${revokedSectionHtml}
|
||||
</div>
|
||||
`;
|
||||
|
||||
Reference in New Issue
Block a user