generated from alex/base-template
Add network issues tolerance #7
Labels
No labels
dependencies
No milestone
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
alex/irc-greeter-bot#7
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
At the moment, in case of a network issue whatsoever when connecting to the server, the process crashes:
We should manage this gracefully instead of crashing. Let's add an infinite loop with some fixed backpressure on it.
/pi new PR
Now I understand the codebase. The issue is that
client.connect()inmain()is awaited directly, so a network failure (likeConnectionRefused) propagates up and crashes the process. I'll add a retry loop with a fixed backoff, extracted as a testableretryForeverhelper, and refactormain()to recreate a fresh client on each attempt.Let me make the changes:Now let me add tests for the retry logic:I used
assertRejectsin the test but need to import it. Let me fix that:Now let me verify the changes and run the tests, lint, and type check:Type check passes. Now let me run tests, lint, and format:Two issues to fix: a lint error (asyncwith noawait) and an off-by-one in my test assertion. Let me look at the test file:All 54 tests pass and lint is clean. Let me verify formatting and run the type check once more:Everything passes. Let me check the README to see if there's a good place to document the new reconnect behavior:I'll add a brief note about the reconnect behavior to the README, after the compile instructions:All checks pass. Let me review the final diff before creating the PR:The diff is clean and all checks pass (type check, lint, 54 tests, format). Let me create the PR:PR #8 created.The bot no longer crashes on transient network failures during the initial IRC connection. Instead, a new
retryForeverhelper wrapsclient.connect()in an infinite loop with a fixed 10-second backoff between failed attempts. Each attempt builds a fresh client (via the extractedcreateClient) so a half-open socket from a prior failure never leaks forward, and failures are logged with the attempt number and backoff interval. The retry logic is covered by three new unit tests (success-first-try, retry-until-success, and never-gives-up), and a README section documents the new behaviour.View action run | Model: zai/glm-5.2 (thinking: max) | Time: 6m 50s | Tokens: 595.6K | Pi SDK v0.80.10 | Action v2.26.0-develop (develop @ c0cad3d)
/pi apparently the PR didn't fix the issue, it's still failing miserably after e.g. IRC server upgrade:
Let's fix it for real this time in a new PR
Let me understand the full codebase structure and the issue better.Now I can see the root cause. Let me read the EventEmitter implementation to understand error handling:Now I have a complete picture of the root cause. Let me verify the key finding:
The
@irc/clientlibrary'sconnect()method catches connection errors internally and returnsnullinstead of throwing:The
retryForeverwrapper only catches thrown errors. Sinceconnect()resolves withnull,retryForeverthinks the operation succeeded and returns — so the retry never happens. The bot proceeds with a dead connection, and the process exits when the event loop empties (no active I/O).Let me now implement the fix:Now I'll implement the fix. The key changes:
connect()return value —@irc/client'sconnect()returnsnullon failure instead of throwing, soretryForevernever detected the failureretryForever— connect, stay alive until disconnected, then throw to trigger reconnectiongracefulShutdownfunction has a bug — theinProgressguard is recreated on each call so it doesn't actually prevent re-entrancy. Let me clean this up with a simpler approach:Now fix the double blank line:Now let me type-check and run existing tests:All tests pass. Let me run the linter:All good. Now let me add a test that specifically documents the bug being fixed —@irc/client'sconnect()returningnullinstead of throwing:Now I'll add a test that documents the specific bug —@irc/client'sconnect()returnsnullinstead of throwing:All 55 tests pass. Let me do a final review of the complete file:Let me also check the README for any documentation that needs updating:Now let me run the complete check suite one more time:Lint caught a missingawaitin the test. Let me fix it to more accurately simulate the asyncconnect()pattern:All checks pass: type-check, lint, format, and 55 tests. Let me create the PR:PR created. Let me verify CI picks it up:The PR is created at #10. Here's a summary of the fix:Why the previous fix didn't work
The
retryForeverwrapper only catches thrown errors. But@irc/client'sClient.connect()never throws on connection failure — it catches the error internally, emits it as an"error"event, and returnsnull:So
retryForeversaw a resolved promise, treated it as success, returned immediately, and the bot proceeded with a dead connection. No active I/O kept the event loop alive → process exited → systemd restart loop →start-limit-hit.This explains the confusing logs where
"Connected and greeting newcomers"appeared before the error —retryForeverhad already returned.What changed
Null check on
connect()return — converts the silentnullinto a thrown error soretryForeveractually retriesFull lifecycle in the retry loop — after connecting, the operation blocks on
await client.once("disconnected"). When the connection drops (e.g. IRC server upgrade), it throws to trigger reconnection with backoffSIGINT handler uses a mutable client reference — works across reconnections instead of holding a stale
ClientView action run | Model: zai/glm-5.2 (thinking: max) | Time: 13m 4s | Tokens: 3.9M | Pi SDK v0.80.10 | Action v2.26.0-develop (develop @ c0cad3d)