Skip to content

Commit 82e1fd2

Browse files
committed
fix: report the swallowed fallback throw, and fix the vacuous slice
Delta review of the previous commit found four things. The shim port's catch was empty, which hid dev-overlay bugs on that one path. A throw out of an EventSource listener (the old shape) and out of the SharedWorker path's onmessage ten lines below both reach the console, so discarding it here was a new behaviour rather than the restored one the comment claimed. It is reported now. The slice meant to anchor that guard's assertion to the fallback function ended at the bootstrap that follows it, so it still trailed a closing brace plus `try {`, leaving the bootstrap's try inside the slice. The first assertion therefore matched with the guard removed, and the comment asserted a counterfactual that did not hold. The slice now ends at the function's own closing brace, with an assertion that the bootstrap is outside it, and each assertion was re-checked to fail with the guard gone. The architecture page's Request Lifecycle said root middleware runs first and put internal endpoints and static files after it. That was already wrong for /__webjs/* before this PR, and the dev /public/* hoist widens it, so two pages on the same site contradicted each other on the fact this PR exists to correct. The blog example's middleware comment wrote the brand as a lowercase code token in prose, which invariant 11 does not allow.
1 parent 90821f3 commit 82e1fd2

4 files changed

Lines changed: 26 additions & 7 deletions

File tree

examples/blog/middleware.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Global middleware. Runs on every app request before webjs routes it.
2+
* Global middleware. Runs on every app request before WebJs routes it.
33
* Return a Response to short-circuit; call next() to continue.
44
*
55
* To add framework sessions:

packages/server/src/dev.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3096,7 +3096,14 @@ function __webjsDirectEvents() {
30963096
try {
30973097
if (m.type === 'reload') __webjsReloadWhenReady();
30983098
else if (m.type === 'webjs-error') __webjsApplyError(m.data);
3099-
} catch (_) { /* a bad frame must not cost this tab its live reload */ }
3099+
} catch (_) {
3100+
// Reported, never swallowed. Detachment is the only thing being
3101+
// prevented: a throw out of an EventSource listener (the old shape) and
3102+
// out of the SharedWorker path's onmessage below both surface to the
3103+
// console, so discarding it here would hide dev-overlay bugs on this one
3104+
// path, which would be a new behaviour rather than a restored one.
3105+
console.error('[webjs] dev reload handler threw', _);
3106+
}
31003107
} }] });
31013108
}
31023109
try {

packages/server/test/dev/reload-shared-connection.test.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,27 @@ test('dev serves the reload SharedWorker, and the client uses it with a direct E
5858
// Sliced to the fallback function's own body first. A regex over the whole
5959
// client matches the SharedWorker bootstrap's `try { ... } catch` further
6060
// down and passes with the guard removed, which is a test that observes
61-
// nothing (found by running exactly that counterfactual).
61+
// nothing.
62+
//
63+
// The slice must end at the function's OWN closing brace, not at the
64+
// bootstrap that follows it: ending at `indexOf('if (typeof SharedWorker')`
65+
// still trails `}\ntry {`, which leaves the bootstrap's `try` inside the
66+
// slice and the first assertion below vacuous again. Verified by running the
67+
// counterfactual against a reconstructed client: each of the three
68+
// assertions below fails with the guard removed.
6269
const fallbackStart = clientSrc.indexOf('function __webjsDirectEvents()');
6370
assert.notEqual(fallbackStart, -1, 'the fallback function is in the client');
64-
const fallbackBody = clientSrc.slice(fallbackStart, clientSrc.indexOf('if (typeof SharedWorker', fallbackStart));
71+
const fallbackEnd = clientSrc.indexOf('\n}\n', fallbackStart);
72+
assert.notEqual(fallbackEnd, -1, 'the fallback function closes');
73+
const fallbackBody = clientSrc.slice(fallbackStart, fallbackEnd + 2);
74+
assert.ok(!/if \(typeof SharedWorker/.test(fallbackBody), 'the slice stops before the bootstrap');
6575
assert.match(
6676
fallbackBody,
6777
/postMessage\(m\)\s*\{[^]*?try\s*\{/,
6878
'the shim port opens a try before running any application code',
6979
);
70-
assert.match(fallbackBody, /\}\s*catch\s*\(_\)/, 'and swallows the throw so the relay cannot drop the tab');
80+
assert.match(fallbackBody, /\}\s*catch\s*\(_\)/, 'and catches the throw so the relay cannot drop the tab');
81+
assert.match(fallbackBody, /console\.error\(/, 'and reports it rather than discarding it');
7182
assert.ok(
7283
fallbackBody.indexOf('try {') < fallbackBody.indexOf('__webjsReloadWhenReady()'),
7384
'the guard opens BEFORE the reload call, not around something else',

website/app/docs/architecture/page.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,10 @@ import { listPosts } from '#modules/posts/queries/list-posts.server.ts';</code-b
103103
<h2>Request Lifecycle</h2>
104104
<ol>
105105
<li><strong>HTTP request arrives</strong> at the Node HTTP server (or HTTP/2 if TLS configured).</li>
106-
<li><strong>Root middleware</strong> (<code>middleware.ts</code>) runs first if present.</li>
106+
<li><strong>Framework-internal assets and probes</strong> (<code>/__webjs/*</code>: the core runtime, the dev reload client, downloaded vendor bundles, <code>/__webjs/health</code> and <code>/__webjs/ready</code>) are served here, ahead of everything below. They depend on neither the app analysis nor the vendor importmap, so a cold instance must not gate them. In <strong>development only</strong>, <code>/public/*</code> plus the <code>/sw.js</code> and <code>/offline.html</code> root remaps and <code>/favicon.ico</code> are served here too, so a stylesheet is never queued behind the startup analysis.</li>
107+
<li><strong>Root middleware</strong> (<code>middleware.ts</code>) runs next if present, for every request that was not already answered above.</li>
107108
<li><strong>103 Early Hints</strong> sent (prod only) with modulepreload URLs for the matched page.</li>
108-
<li><strong>Route matching</strong>: the router tries (in order) internal endpoints, static files, user source modules, API routes (<code>route.ts</code>), then page routes.</li>
109+
<li><strong>Route matching</strong>: the router tries (in order) static files, user source modules, API routes (<code>route.ts</code>), then page routes. In production this is where <code>/public/*</code> is served, so a middleware that guards an asset still guards it.</li>
109110
<li><strong>Segment middleware</strong> chain runs (outermost → innermost) for the matched route.</li>
110111
<li>For <strong>pages</strong>: SSR pipeline runs (load page + layouts, render to HTML, inject DSD, collect metadata, stream response with Suspense).</li>
111112
<li>For <strong>API routes</strong>: the matched handler function runs, returns a Response.</li>

0 commit comments

Comments
 (0)