fix: remove contradictory process.exit unhandledRejection handler - #1457
fix: remove contradictory process.exit unhandledRejection handler#1457ionfwsrijan wants to merge 1 commit into
Conversation
Two global unhandledRejection listeners existed with opposite intents: the first logged and kept the server running, the second called process.exit(1), so any unhandled rejection killed the whole process. Remove the exit-based handler and keep the single log-and-continue policy. Closes Canopus-Labs#1441
📝 WalkthroughWalkthroughThe duplicate ChangesUnhandled rejection policy
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/tests/unhandledRejection.policy.unit.test.js`:
- Around line 16-25: The tests in unhandledRejection.policy.unit.test.js must
validate runtime behavior instead of matching backend/server.js source text.
Move the unhandledRejection registration into a side-effect-free module, import
it in the test, inspect process.listeners("unhandledRejection") to verify one
registered listener, invoke that listener, and spy on console.error and
process.exit to assert the required logging and non-exit policy without starting
the server.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 74e0ad48-1d55-4ec3-9574-112a1dfd3841
📒 Files selected for processing (2)
backend/server.jsbackend/tests/unhandledRejection.policy.unit.test.js
💤 Files with no reviewable changes (1)
- backend/server.js
| const matches = serverSource.match(/process\.on\(\s*["']unhandledRejection["']/g) || []; | ||
| expect(matches.length).toBe(1); | ||
| }); | ||
|
|
||
| it("does not exit the process from the unhandledRejection handler", () => { | ||
| const listenerBlock = serverSource.slice( | ||
| serverSource.indexOf('"unhandledRejection"'), | ||
| serverSource.indexOf("unhandledRejection") + 300 | ||
| ); | ||
| expect(listenerBlock).not.toMatch(/process\.exit/); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- backend/tests/unhandledRejection.policy.unit.test.js ---'
cat -n backend/tests/unhandledRejection.policy.unit.test.js
printf '%s\n' '--- backend/server.js relevant lines ---'
nl -ba backend/server.js | sed -n '1,80p'
printf '%s\n' '--- references to the policy and test setup ---'
rg -n --glob '!node_modules' 'unhandledRejection|console\.error|process\.exit|serverSource' backend package.json 2>/dev/null | head -200Repository: Canopus-Labs/PrepPilot
Length of output: 10123
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- backend/server.js lines 1-25 and 175-205 ---'
sed -n '1,25p;175,205p' backend/server.js
printf '%s\n' '--- project module and test configuration ---'
for f in package.json backend/package.json vitest.config.js backend/vitest.config.js; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat "$f"
fi
done
printf '%s\n' '--- server.js imports and startup calls ---'
rg -n '^(import|const .*require)|listen\(|connect\(|start|app\.' backend/server.js | head -120Repository: Canopus-Labs/PrepPilot
Length of output: 5718
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
test_source = Path("backend/tests/unhandledRejection.policy.unit.test.js").read_text(encoding="utf-8-sig")
server_source = Path("backend/server.js").read_text(encoding="utf-8")
pattern = re.compile(r'''process\.on\(\s*["']unhandledRejection["']''')
def assertions(source):
matches = pattern.findall(source)
start = source.find('"unhandledRejection"')
end = source.find("unhandledRejection") + 300
block = source[start:end]
return len(matches), start, end, "process.exit" in block
variants = {
"current": server_source,
"single-quoted event": server_source.replace(
'process.on("unhandledRejection"', "process.on('unhandledRejection'"
),
"uses process.once": server_source.replace(
'process.on("unhandledRejection"', 'process.once("unhandledRejection"'
),
"exit after window": server_source.replace(
' console.error("Unhandled Promise Rejection:", err);',
' console.error("Unhandled Promise Rejection:", err);\\n' + " " * 400 + "process.exit(1);",
),
"missing console.error": server_source.replace(
' console.error("Unhandled Promise Rejection:", err);\\n', ""
),
}
print("test source uses runtime listener APIs:", bool(re.search(
r'process\.(listeners|once|addListener)|import\s+.*server', test_source
)))
for name, source in variants.items():
count, start, end, has_exit = assertions(source)
print(f"{name}: matches={count}, start={start}, end={end}, detects_exit={has_exit}")
server_lines = server_source.splitlines()
print("server starts listening:", any("app.listen(" in line for line in server_lines))
print("policy logs:", any("Unhandled Promise Rejection:" in line for line in server_lines))
PYRepository: Canopus-Labs/PrepPilot
Length of output: 594
Test the registered policy at runtime, not with source-text matching.
The current tests can pass when the handler uses process.once, does not register, omits console.error, or calls process.exit outside the 300-character slice. Importing backend/server.js starts the server, so move the registration to a side-effect-free module, inspect process.listeners("unhandledRejection"), invoke the listener, and spy on console.error and process.exit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/tests/unhandledRejection.policy.unit.test.js` around lines 16 - 25,
The tests in unhandledRejection.policy.unit.test.js must validate runtime
behavior instead of matching backend/server.js source text. Move the
unhandledRejection registration into a side-effect-free module, import it in the
test, inspect process.listeners("unhandledRejection") to verify one registered
listener, invoke that listener, and spy on console.error and process.exit to
assert the required logging and non-exit policy without starting the server.
Problem
server.jsregisters two globalunhandledRejectionlisteners with opposite intents:process.exit(1), killing the entire server.Because both are attached to the same event, any unhandled rejection kills the whole process — contradicting the intent of the first handler.
Fix
Removed the second,
process.exit(1)-based handler. Exactly one log-and-continueunhandledRejectionpolicy now remains;uncaughtExceptionstill exits deliberately as before.Files changed
backend/server.js— removed the exit-basedunhandledRejectionhandler.backend/tests/unhandledRejection.policy.unit.test.js— new test asserting exactly oneunhandledRejectionlistener exists and it does not callprocess.exit.Testing
npx vitest run tests/unhandledRejection.policy.unit.test.js— 2/2 passing.Closes #1441
Removes the duplicate
unhandledRejectionhandler and preserves the log-and-continue policy. Adds tests that verify one listener exists and that it does not callprocess.exit.