fix(cli): guarantee non-empty [STARTUP] Fatal log on instrumentation-hook boot throw - #10447
Open
diegosouzapw wants to merge 1 commit into
Open
fix(cli): guarantee non-empty [STARTUP] Fatal log on instrumentation-hook boot throw#10447diegosouzapw wants to merge 1 commit into
diegosouzapw wants to merge 1 commit into
Conversation
…hook boot throw Refs #10171: on native Windows / WSL2 boots, an instrumentation-hook throw during module-load or registerNodejs() leaves the HTTP listener up while every DB-touching route 500s, with app.log staying completely empty. The #7773/#7828 guard in ensureDbReadyForBoot only logs one specific failure class (DB driver init). register() in src/instrumentation.ts now wraps the boot call in a try/catch at the outermost boundary and unconditionally logs a "[STARTUP] Fatal: instrumentation hook failed during boot:" line before rethrowing, so app.log/stdout is never silently empty on a failed boot regardless of platform or which step threw. This is a partial diagnostic hardening, not the full fix for #10171 — the platform-specific root cause on native Windows/WSL2 still needs the reporter's raw child stderr from a real host (tracked separately, see _tasks/pipeline/bugs/2-implementing/10171-instrumentation-hook-500-on-windows-wsl.plan.md).
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
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.
Summary
Partial diagnostic hardening for #10171 (native Windows / WSL2 boot failure — instrumentation
hook crashes, HTTP listener stays up, every DB-touching route 500s,
app.logstays completelyempty). Per the plan-file (
_tasks/pipeline/bugs/2-implementing/10171-instrumentation-hook-500-on-windows-wsl.plan.md,verdict
needs-vps), the true root cause on native Windows/WSL2 requires capturing raw childstderr on a real host — that part is still open and out of scope here.
This PR implements the one safe, narrowly-scoped, testable improvement identified in the plan's
implementation checklist that is independent of the unknown platform-specific root cause:
src/instrumentation-node.ts::ensureDbReadyForBoot(#7773/#7828) already prints[STARTUP] Fatal: Database driver initialization failed:for one specific failure class (DBdriver init), but nothing guaranteed a fatal throw ANYWHERE during instrumentation-hook boot —
including a throw that happens before
ensureDbReadyForBootis even reached (e.g. a module-loaderror importing
./instrumentation-nodeitself) — produces a diagnostic line.register()insrc/instrumentation.tsnow wraps the boot call in a try/catch at the outermost boundary andunconditionally logs a
[STARTUP] Fatal: instrumentation hook failed during boot:line to stdoutbefore rethrowing, so
app.log/stdout is never silently empty on a failed boot, regardless ofplatform or which step threw.
Explicitly not fixed here (needs the reporter's raw stderr from a real Windows/WSL2 host,
per the plan file):
better-sqlite3/node:sqlite/sql.js) fix specific to that platform.Out of scope, confirmed correct, untouched:
needsAndroidCacheDirPrep/isTermux(
bin/cli/utils/ensureAndroidCacheDir.mjs,scripts/build/postinstallSupport.mjs).Regression test
tests/unit/instrumentation-hook-boot-fatal-log-10171.test.ts— 3 cases:Error) during instrumentation-hook boot is logged with a non-empty[STARTUP] Fatal:line containing the failure message, before rethrow.Errorthrow (e.g. barethrow "string", mirroringsql.js's fix(startup): Cannot create property 'message' on string 'Database closed' at registerInstrumentation #6560 pattern) is alsologged.
Fail → pass evidence (ran against the pre-fix
register(), then the fixed version):Gates run
npm run typecheck:core— exit 0npx eslint --suppressions-location config/quality/eslint-suppressions.json src/instrumentation.ts tests/unit/instrumentation-hook-boot-fatal-log-10171.test.ts— clean, no outputnode scripts/check/check-file-size.mjs—src/instrumentation.tsis 42 lines, no new god-filenode scripts/check/check-test-discovery.mjs— OK, new test file discoveredeslint.complexity-ratchets.config.mjs) scoped tosrc/instrumentation.ts— zero violationsnode --import tsx/esm --test tests/unit/instrumentation-hook-boot-fatal-log-10171.test.ts— 3/3 passRefs #10171 (not Closes — this is a partial diagnostic hardening; the platform-specific root
cause still needs a real Windows/WSL2 host's raw child stderr per the plan file, tracked as
hold-vps).to this change.