fix(discord): suppress intentional reconnect exception during health monitor restart

[Bartok9]

Summary

When the Discord health monitor triggers a restart (e.g., due to stale-socket detection), the onAbort handler sets maxAttempts to 0 before calling disconnect(). The gateway library interprets this as a reconnection failure and throws "Max reconnect attempts (0) reached" — causing an uncaught exception that crashes the entire gateway process.

Root Cause

The gateway library's disconnect flow treats maxAttempts: 0 as a signal that reconnection exhausted its attempts, throwing an error. However, in the health monitor's abort flow, maxAttempts: 0 is intentionally set to prevent reconnection during a clean shutdown — this exception should not bubble up.

Fix

Wrap the gateway.disconnect() call in a try-catch that:

1. Suppresses the expected "Max reconnect attempts" error (this is an intentional abort, not a connection failure)
2. Rethrows any other unexpected errors (preserves visibility into real issues)

Testing

Added two test cases:

• suppresses 'Max reconnect attempts' exception during intentional abort — verifies the fix
• rethrows non-reconnect exceptions during abort — ensures we don't suppress legitimate errors

All 17 tests in provider.lifecycle.test.ts pass.

Impact

• Before: Gateway crashes every ~30 minutes when health monitor triggers restart
• After: Health monitor gracefully restarts Discord connection without crashing

Fixes #55026

[greptile-apps]

Greptile Summary

This PR fixes a recurring gateway process crash (~every 30 minutes) caused by the health monitor's onAbort handler setting maxAttempts: 0 before calling gateway.disconnect(). The gateway library interprets that combination as exhausted reconnection attempts and throws "Max reconnect attempts (0) reached", which previously bubbled up as an uncaught exception.\n\nThe fix wraps gateway.disconnect() in a targeted try-catch inside onAbort that suppresses only the expected "Max reconnect attempts" error and rethrows everything else, preserving visibility into genuine failures. Two new test cases validate both branches.\n\nKey changes:\n- provider.lifecycle.ts — adds try-catch in onAbort around gateway.disconnect() with selective suppression based on error message content.\n- provider.lifecycle.test.ts — two new tests: one confirming the known error is suppressed, one confirming unexpected errors still propagate.\n\nThe fix is minimal, well-scoped, and correctly targeted at the root cause. One minor style concern: the error match uses String(err) rather than err.message, and the sentinel string is inlined — see the inline comment.

Confidence Score: 5/5

Safe to merge — the fix correctly suppresses an intentional exception without masking real errors, and both new test cases pass.

The change is small, targeted, and well-tested. The root cause (gateway library throwing on maxAttempts: 0) is clearly documented and the suppression logic is tight. The only open item is a P2 style suggestion about using err.message vs String(err), which does not affect correctness.

No files require special attention.

Important Files Changed

Filename	Overview
extensions/discord/src/monitor/provider.lifecycle.ts	Wraps gateway.disconnect() in onAbort with a targeted try-catch that suppresses the expected 'Max reconnect attempts (0) reached' exception while rethrowing anything else. The logic is correct and well-scoped.
extensions/discord/src/monitor/provider.lifecycle.test.ts	Adds two well-structured tests covering both the suppression path and the rethrow path. Uses the pre-aborted signal pattern to exercise onAbort() synchronously, which is appropriate for these scenarios.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/discord/src/monitor/provider.lifecycle.ts
Line: 139-142

Comment:
**Fragile string-match for error detection**

The suppression logic relies on a partial string match against the serialised error. Two minor concerns:

1. `String(err)` on an `Error` instance yields `"Error: Max reconnect attempts …"`, so the match works today — but using `err instanceof Error ? err.message : String(err)` makes the intent clearer and avoids accidentally matching a hypothetical `Error: Something wrapping 'Max reconnect attempts'` object.

2. If the upstream gateway library ever changes its error message wording, the condition silently stops suppressing, and the crash resurfaces with no warning. Centralising the sentinel string in a named constant (e.g. `const MAX_RECONNECT_ERROR_PREFIX = "Max reconnect attempts"`) makes future auditing easier.

```suggestion
      const message = err instanceof Error ? err.message : String(err);
      if (!message.includes("Max reconnect attempts")) {
        throw err;
      }
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(discord): suppress intentional recon..." | Re-trigger Greptile}

[greptile-apps]

Fragile string-match for error detection

The suppression logic relies on a partial string match against the serialised error. Two minor concerns:

1. String(err) on an Error instance yields "Error: Max reconnect attempts …", so the match works today — but using err instanceof Error ? err.message : String(err) makes the intent clearer and avoids accidentally matching a hypothetical Error: Something wrapping 'Max reconnect attempts' object.

2. If the upstream gateway library ever changes its error message wording, the condition silently stops suppressing, and the crash resurfaces with no warning. Centralising the sentinel string in a named constant (e.g. const MAX_RECONNECT_ERROR_PREFIX = "Max reconnect attempts") makes future auditing easier.

Suggested change

	const message = String(err);
	if (!message.includes("Max reconnect attempts")) {
	throw err;
	}
	const message = err instanceof Error ? err.message : String(err);
	if (!message.includes("Max reconnect attempts")) {
	throw err;
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: extensions/discord/src/monitor/provider.lifecycle.ts
Line: 139-142

Comment:
**Fragile string-match for error detection**

The suppression logic relies on a partial string match against the serialised error. Two minor concerns:

1. `String(err)` on an `Error` instance yields `"Error: Max reconnect attempts …"`, so the match works today — but using `err instanceof Error ? err.message : String(err)` makes the intent clearer and avoids accidentally matching a hypothetical `Error: Something wrapping 'Max reconnect attempts'` object.

2. If the upstream gateway library ever changes its error message wording, the condition silently stops suppressing, and the crash resurfaces with no warning. Centralising the sentinel string in a named constant (e.g. `const MAX_RECONNECT_ERROR_PREFIX = "Max reconnect attempts"`) makes future auditing easier.

```suggestion
      const message = err instanceof Error ? err.message : String(err);
      if (!message.includes("Max reconnect attempts")) {
        throw err;
      }
```

How can I resolve this? If you propose a fix, please make it concise.

[Bartok9]

Addressed the style feedback — now using err instanceof Error ? err.message : String(err) for cleaner message extraction. The matching behavior is unchanged since we use .includes(), but the explicit type guard makes the intent clearer.

[ryancush]

This fix is needed — the stale-socket health monitor crash is hitting production users on 2026.3.24. The fix looks correct (try-catch around disconnect() to suppress the expected Max reconnect attempts error on intentional aborts). Happy to help test. Is there anything blocking merge?

[steipete]

Closing this as implemented after Codex review.

Current main already covers this Discord reconnect-exhaustion crash with a broader lifecycle/supervisor rewrite, so the open PR is superseded.

What I checked:

• Abort path now disconnects under retained lifecycle handlers: waitForDiscordGatewayStop resolves aborts by calling gateway.disconnect() before detaching listeners, specifically to keep late error events from becoming uncaught during disconnect. (extensions/discord/src/monitor.gateway.ts:18, 6a61cb73c562)
• Late reconnect-exhausted errors are explicitly classified and suppressed during teardown/dispose: The gateway supervisor classifies "Max reconnect attempts" as reconnect-exhausted, and in teardown/disposed phases it logs and suppresses those late events instead of letting them escape. (extensions/discord/src/monitor/gateway-supervisor.ts:76, 6a61cb73c562)
• Abort behavior is covered by tests on main: waitForDiscordGatewayStop has a direct test that abort resolves, disconnects the gateway, and detaches lifecycle handlers cleanly. (extensions/discord/src/monitor.gateway.test.ts:80, 6a61cb73c562)
• Disconnect-emitted errors during abort are handled without changing the abort outcome: There is explicit coverage that the lifecycle handler stays active until disconnect() returns on abort, so errors emitted during disconnect are handled instead of crashing the process. (extensions/discord/src/monitor.gateway.test.ts:143, 6a61cb73c562)
• Provider-level regression coverage exists for the exact symptom class: Provider and supervisor tests assert that late Max reconnect attempts (0) errors after dispose/teardown do not throw and are only logged as suppressed. (extensions/discord/src/monitor/provider.test.ts:281, 6a61cb73c562)

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review notes: reviewed against 34896839ba22; fix evidence: commit 6a61cb73c562.