fix(desktop): stop logging a handler stack for expected session-probe 404s - #79266
Open
hernanda-git wants to merge 1 commit into
Open
Conversation
… 404s
resolveStoredSession() is a probe ladder — cache, then a by-id GET on the
active backend, then the same GET scoped to each other profile until one
hits. A miss on rungs 2 and 3 is expected and already handled: the backend
answers 404 {"detail":"Session not found"} and the renderer falls to the
next rung.
But ipcMain.handle logs "Error occurred in handler for 'hermes:api'" with a
full stack for every rejected invoke, with no per-handler opt-out — so each
designed fall-through printed a multi-line stack on startup and on every
session switch, burying real hermes:api failures.
Extract the handler body, and for that one expected case resolve with a
sentinel instead of rejecting; preload — our own code on the other side of
the same seam — rethrows it as an Error with the identical '404: <body>'
message. The renderer contract is byte-identical (the ladder and its tests
are untouched); only Electron's logging is bypassed. Every other failure
still rejects and still logs in full.
The sentinel check is deliberately narrow (a plain object whose only key is
the sentinel, with a string value) so no backend payload can be mistaken
for one.
Fixes NousResearch#79265
alt-glitch
added
type/bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #79265
Symptom
hermes desktopprints a full stack trace for a routine, already-handled session lookup — several times on startup, and again on every session switch:Nothing is broken — but it reads as a crash, and it buries genuine
hermes:apihandler failures in the same stream.Root cause
Session resolution is a deliberate probe ladder —
resolveStoredSession()inapps/desktop/src/app/session/hooks/use-session-actions/utils.ts:GET /api/sessions/{id}on the active backend;A miss on rungs 2 and 3 is the ladder working as designed. The backend correctly answers
404 {"detail":"Session not found"}(hermes_cli/web_routers/sessions.py), and the renderer'scatchexists precisely to fall to the next rung — the code even says so:fetchSessionLinkTitledocuments the same 404 as "no title".The noise is a pure transport artifact:
ipcMain.handlelogsError occurred in handler for '<channel>'with a full stack for every rejected invoke, and Electron has no per-handler opt-out. So a designed fall-through is reported as a handler crash. More profiles → more probes → more stack traces.The fix
Extract the handler body into
handleHermesApiRequest, and for that one expected case resolve with a sentinel instead of rejecting.preload.ts— our own code on the other side of the same seam — rethrows it as anErrorcarrying the identical404: <body>message.The renderer contract is byte-identical: the ladder, its
catchblocks, and its existing tests are untouched. Only Electron's default handler logging is bypassed, and only for this case. Every other failure rejects as before and still logs in full.The shared literal and both helpers live in a new
electron/api-expected-404.tsso main and preload can't drift apart, with the reasoning documented at the top of the file.Why not just swallow it in main? The renderer must still see a rejection — the ladder branches on it. Returning
nullwould make "not on this profile" indistinguishable from a real empty response.Why is the sentinel check narrow?
isExpectedNotFoundSentinelaccepts only a plain object whose only key is the sentinel and whose value is a string, so no backend payload (including one that happened to carry that key alongside real data) can be mistaken for one.Tests
New
apps/desktop/electron/api-expected-404.test.ts(5 tests):null, arrays, a wrapped sentinel, wrong value type, right key with real data alongside — are all rejected;404-shaped probe predicate — the seam can't silently change what the ladder sees.Existing consumers still green:
Full electron project: 18 failing on
main→ 17 with this branch (+5 new passing). The remaining failures are pre-existing and unrelated to this change (ssh-config,ssh-connection,wsl-path-bridge,windows-hermes-path,before-pack,stage-native-deps— POSIX/env-dependent tests that fail on a Windows checkout).npx tsc -p tsconfig.electron.json --noEmitis clean.Verified on Windows 10.
Note
Same log excerpt as #79245 / #79250, but a genuinely separate bug — that one is simple-git warning spam from the git review ops; this one is IPC error logging for a designed fall-through. Kept as separate issues and separate PRs so each can be judged on its own merits. This branch is cut from
mainand does not depend on #79250.