mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
SidandGitHub
362b92c1a6
* fix(packaged): swallow harmless setTypeOfService EINVAL from undici (#895) Opening Settings → Pets → Community in the packaged desktop app surfaced a native "JavaScript error in main process" dialog with `Uncaught Exception: Error: setTypeOfService EINVAL`. Root cause: undici's socket setup tries to set the IP_TOS byte for QoS / DSCP marking on outbound sockets, and the macOS kernel refuses with EINVAL on certain configurations (VPNs, IPv6-only sockets, some firewall postures). The byte is purely advisory — the socket itself is healthy and serves traffic — so the rejection should not crash the app. Two cooperating layers: 1. **`protocol.ts`** registers the `od://` scheme that backs every renderer page load and API call in the packaged build by forwarding through Node's global `fetch` (which is undici under the hood). Pulled the inner request handler out as `handleOdRequest()` so a test can drive it with a stub fetch, and wrapped the `await fetch()` in a try/catch that returns a 502 Response on failure. Without this, every undici rejection — not just `setTypeOfService` — propagated to Electron's default uncaught-exception path. Now the renderer sees a normal error response and the main process keeps running. 2. **`logging.ts`** adds a defensive `process.on('uncaughtException')` handler with a narrow filter, `isHarmlessSocketOptionError`, that only matches the canonical undici shape (message contains `setTypeOfService` AND code is `EINVAL` or message contains `EINVAL`). For any unrecognised error the handler re-throws via `setImmediate` so Node's default crash + Electron's crash dialog still fire end-to-end — a future regression that broadens the filter to "every EINVAL" is caught by the unit tests below. Tests: 13 new tests across `tests/protocol.test.ts` (5) and `tests/logging.test.ts` (8) pin both layers — including the explicit #895 regression case (fetch rejecting with the canonical EINVAL shape returns a 502 instead of throwing) and the negative guard against the filter swallowing real bugs (a generic write EINVAL or a setTypeOfService EACCES is *not* matched). Verified locally: - `pnpm --filter @open-design/packaged vitest tests/protocol.test.ts tests/logging.test.ts` → 13/13 - packaged `tsconfig.json` and `tsconfig.tests.json` (the CI killer): both clean - the one pre-existing failure in `tests/sidecars.test.ts` (`adds custom VP_HOME/bin …`) is independent of this PR — confirmed by stashing the change and re-running * fix(packaged): break recursive rethrow + tighten EINVAL filter (#906 review) @mrcfps and @lefarcen both flagged a real P1 in the first iteration: the non-harmless branch of the new uncaughtException handler rethrew via setImmediate while the same listener was still registered, so a real bug would re-enter the handler indefinitely instead of terminating. mrcfps reproduced the loop with a minimal Node script. lefarcen also flagged that the filter trusted the message string over a contradicting structured `code`. Both fixes: 1. **Recursive rethrow (P1).** Extract the handler as a named factory, `createFatalUncaughtExceptionHandler(logger)`, that captures itself in closure. On non-harmless errors the handler now `process.removeListener('uncaughtException', self)` before scheduling the rethrow. With no listener registered, the next throw lands in Node's default crash path — which is exactly what we wanted ("preserve fail-fast for real bugs"). 2. **`code` is authoritative (P2).** When `code` is present on the error, only `code === 'EINVAL'` qualifies. A contradicting `EACCES`/`EPERM` paired with `setTypeOfService EINVAL` in the message now slips through to the crash path instead of being swallowed. Message-based detection only fires when `code` is genuinely absent (some libuv builds don't populate it on raw thrown Errors). 3 new tests pin both fixes: - `does NOT match when code contradicts the message` and the EPERM variant guard against the P2 regression. - `removes itself from uncaughtException listeners before scheduling the rethrow` uses `vi.spyOn(process, 'removeListener')` and a stubbed setImmediate to assert the call order: removeListener fires before setImmediate schedules the throw. - `does NOT re-enter itself when invoked twice` is a belt-and-suspenders loop guard — even if a future refactor dropped the removeListener call, the test would catch runaway scheduling. Verified locally: - packaged vitest: 18/18 (was 13, +3 new tests; +2 negative-guard tests for the P2 filter; -0 deletions) - packaged tsc -p tsconfig.json --noEmit: clean - packaged tsc -p tsconfig.tests.json --noEmit (the CI killer): clean
88 lines
2.7 KiB
TypeScript
88 lines
2.7 KiB
TypeScript
import { protocol } from "electron";
|
|
|
|
const OD_SCHEME = "od";
|
|
const OD_ENTRY_URL = `${OD_SCHEME}://app/`;
|
|
|
|
protocol.registerSchemesAsPrivileged([
|
|
{
|
|
privileges: {
|
|
corsEnabled: true,
|
|
secure: true,
|
|
standard: true,
|
|
stream: true,
|
|
supportFetchAPI: true,
|
|
},
|
|
scheme: OD_SCHEME,
|
|
},
|
|
]);
|
|
|
|
function toWebRuntimeUrl(webRuntimeUrl: string, requestUrl: string): string {
|
|
const incoming = new URL(requestUrl);
|
|
const target = new URL(webRuntimeUrl);
|
|
target.pathname = incoming.pathname;
|
|
target.search = incoming.search;
|
|
target.hash = incoming.hash;
|
|
return target.toString();
|
|
}
|
|
|
|
function buildProxyErrorResponse(error: unknown, target: string): Response {
|
|
const message = error instanceof Error ? error.message : String(error);
|
|
const code =
|
|
error instanceof Error && typeof (error as NodeJS.ErrnoException).code === "string"
|
|
? (error as NodeJS.ErrnoException).code
|
|
: null;
|
|
return new Response(
|
|
JSON.stringify({
|
|
error: "OD_PROTOCOL_PROXY_FAILED",
|
|
message,
|
|
...(code === null ? {} : { code }),
|
|
target,
|
|
}),
|
|
{
|
|
status: 502,
|
|
headers: { "content-type": "application/json" },
|
|
},
|
|
);
|
|
}
|
|
|
|
/**
|
|
* Inner request handler for the `od://` Electron protocol — every
|
|
* renderer fetch flows through here and gets proxied to the local web
|
|
* sidecar via Node's global `fetch` (which is undici under the hood).
|
|
*
|
|
* Pulled out as a named export so unit tests can drive it with a stub
|
|
* `fetchImpl` without spinning up Electron, and so the try/catch
|
|
* stays auditable from one place.
|
|
*
|
|
* Why the try/catch matters: undici can throw `setTypeOfService
|
|
* EINVAL` from socket internals on certain macOS / VPN configurations
|
|
* (issue #895). Without the catch, the rejection bubbles all the way
|
|
* up to the Electron main process and surfaces as a native
|
|
* "JavaScript error in main process" dialog the next time the user
|
|
* does anything that triggers a renderer-to-sidecar fetch (e.g.
|
|
* Settings → Pets → Community). Returning a 502 instead lets the
|
|
* renderer see a normal failure and keeps the process alive.
|
|
*/
|
|
export async function handleOdRequest(
|
|
request: Request,
|
|
webRuntimeUrl: string,
|
|
fetchImpl: typeof fetch = fetch,
|
|
): Promise<Response> {
|
|
const target = toWebRuntimeUrl(webRuntimeUrl, request.url);
|
|
try {
|
|
return await fetchImpl(new Request(target, request));
|
|
} catch (error) {
|
|
return buildProxyErrorResponse(error, target);
|
|
}
|
|
}
|
|
|
|
export function packagedEntryUrl(): string {
|
|
return OD_ENTRY_URL;
|
|
}
|
|
|
|
export function registerOdProtocol(webRuntimeUrl: string): void {
|
|
protocol.handle(OD_SCHEME, async (request) => {
|
|
return await handleOdRequest(request, webRuntimeUrl);
|
|
});
|
|
}
|