fix(discord): catch unhandled promise rejection in gateway registerClient (#34592)#35145
Closed
lml2468 wants to merge 2 commits into
Closed
fix(discord): catch unhandled promise rejection in gateway registerClient (#34592)#35145lml2468 wants to merge 2 commits into
lml2468 wants to merge 2 commits into
Conversation
openclaw-barnacle
Bot
added
channel: discord
Contributor
Greptile SummaryThis PR fixes a process-crashing unhandled promise rejection in the Discord gateway by wrapping Key changes:
Minor concern: Confidence Score: 4/5
Last reviewed commit: 8c7fd39 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+76
to
+161
| @@ -82,6 +140,24 @@ export function createDiscordGatewayPlugin(params: { | |||
| return new ProxyGatewayPlugin(); | |||
| } catch (err) { | |||
| params.runtime.error?.(danger(`discord: invalid gateway proxy: ${String(err)}`)); | |||
| return new GatewayPlugin(options); | |||
|
|
|||
| // Fall back to a safe non-proxy plugin. | |||
| class SafeGatewayPlugin extends GatewayPlugin { | |||
| constructor() { | |||
| super(options); | |||
| } | |||
|
|
|||
| override async registerClient( | |||
| client: Parameters<GatewayPlugin["registerClient"]>[0], | |||
| ): Promise<void> { | |||
| try { | |||
| await super.registerClient(client); | |||
| } catch (error) { | |||
| logRegisterClientFailure(error, params.runtime); | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| return new SafeGatewayPlugin(); | |||
Contributor
There was a problem hiding this comment.
Duplicated SafeGatewayPlugin class definition
SafeGatewayPlugin is defined with identical logic in two separate branches: the !proxy path (lines 76–90) and the invalid-proxy catch fallback (lines 145–159). If this behaviour ever needs to change, both copies must be updated in sync.
Consider extracting a single shared factory before the if (!proxy) block:
function makeSafePlugin() {
class SafeGatewayPlugin extends GatewayPlugin {
constructor() {
super(options);
}
override async registerClient(
client: Parameters<GatewayPlugin["registerClient"]>[0],
): Promise<void> {
try {
await super.registerClient(client);
} catch (error) {
logRegisterClientFailure(error, params.runtime);
}
}
}
return new SafeGatewayPlugin();
}Then both branches simply call return makeSafePlugin();.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/monitor/gateway-plugin.ts
Line: 76-161
Comment:
**Duplicated `SafeGatewayPlugin` class definition**
`SafeGatewayPlugin` is defined with identical logic in two separate branches: the `!proxy` path (lines 76–90) and the invalid-proxy `catch` fallback (lines 145–159). If this behaviour ever needs to change, both copies must be updated in sync.
Consider extracting a single shared factory before the `if (!proxy)` block:
```ts
function makeSafePlugin() {
class SafeGatewayPlugin extends GatewayPlugin {
constructor() {
super(options);
}
override async registerClient(
client: Parameters<GatewayPlugin["registerClient"]>[0],
): Promise<void> {
try {
await super.registerClient(client);
} catch (error) {
logRegisterClientFailure(error, params.runtime);
}
}
}
return new SafeGatewayPlugin();
}
```
Then both branches simply call `return makeSafePlugin();`.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+141
to
+150
| beforeEach(() => { | ||
| baseRegisterClientSpy.mockClear(); | ||
| baseRegisterClientSpy.mockReset(); | ||
| undiciFetchMock.mockClear(); | ||
| undiciFetchMock.mockReset(); | ||
| undiciProxyAgentSpy.mockClear(); | ||
| wsProxyAgentSpy.mockClear(); | ||
| webSocketSpy.mockClear(); | ||
| resetLastAgent(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Redundant mockClear() before mockReset()
mockReset() is a superset of mockClear() — it clears call history and resets the implementation. Calling mockClear() immediately before mockReset() on the same spy is a no-op and just adds noise.
Suggested change
| beforeEach(() => { | |
| baseRegisterClientSpy.mockClear(); | |
| baseRegisterClientSpy.mockReset(); | |
| undiciFetchMock.mockClear(); | |
| undiciFetchMock.mockReset(); | |
| undiciProxyAgentSpy.mockClear(); | |
| wsProxyAgentSpy.mockClear(); | |
| webSocketSpy.mockClear(); | |
| resetLastAgent(); | |
| }); | |
| beforeEach(() => { | |
| baseRegisterClientSpy.mockReset(); | |
| undiciFetchMock.mockReset(); | |
| undiciProxyAgentSpy.mockClear(); | |
| wsProxyAgentSpy.mockClear(); | |
| webSocketSpy.mockClear(); | |
| resetLastAgent(); | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/monitor/gateway-plugin.crash-guard.test.ts
Line: 141-150
Comment:
**Redundant `mockClear()` before `mockReset()`**
`mockReset()` is a superset of `mockClear()` — it clears call history *and* resets the implementation. Calling `mockClear()` immediately before `mockReset()` on the same spy is a no-op and just adds noise.
```suggestion
beforeEach(() => {
baseRegisterClientSpy.mockReset();
undiciFetchMock.mockReset();
undiciProxyAgentSpy.mockClear();
wsProxyAgentSpy.mockClear();
webSocketSpy.mockClear();
resetLastAgent();
});
```
How can I resolve this? If you propose a fix, please make it concise.
…ient (openclaw#34592) Carbon's Client constructor calls plugin.registerClient() without awaiting the returned promise. When Discord API is unreachable (e.g., blocked regions without proxy), the fetch failure becomes an unhandled promise rejection that crashes the entire gateway process in an infinite restart loop. Changes: - SafeGatewayPlugin: wrap super.registerClient() in try/catch for both proxy and non-proxy code paths - ProxyGatewayPlugin: catch proxy fetch failure gracefully, fall back to default gateway URL instead of throwing - FALLBACK_GATEWAY_INFO: provide sensible defaults (url, shards, session_start_limit) so Carbon's reconnect logic can still attempt WebSocket connections with exponential backoff - Invalid proxy fallback also returns SafeGatewayPlugin instead of bare GatewayPlugin AI-assisted (OpenClaw agent, fully tested)
…GatewayPlugin class DRY: the identical SafeGatewayPlugin class was defined twice (no-proxy path and invalid-proxy fallback). Extract into a shared factory function createSafeGatewayPlugin(options, runtime) that both paths now call.
lml2468
force-pushed
the
fix/issue-34592-discord-gateway-crash
branch
from
March 12, 2026 08:35
eafb9a7 to
dcba7ac
Compare
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
1 participant
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.
What
Catch unhandled promise rejections in
GatewayPlugin.registerClient()that crash the entire gateway process when the Discord API is unreachable.Why
Fixes #34592 — When Discord API is unreachable (e.g., blocked regions without proxy/TUN),
registerClientthrows during the gateway info fetch. Carbon'sClientconstructor callsplugin.registerClient(this)without awaiting the returned promise, so any rejection becomes an unhandled promise rejection that kills the Node.js process. This causes the gateway to crash and restart every ~3 seconds in an infinite loop, taking down all channels (Telegram, etc.) along with it.How
SafeGatewayPlugin: New subclass wrappingsuper.registerClient()in try/catch for both proxy and non-proxy code paths. Failures are logged as errors instead of crashing the process.FALLBACK_GATEWAY_INFO: When proxy fetch fails, provide sensible defaults (url,shards,session_start_limit) so Carbon's reconnect logic can still attempt WebSocket connections with exponential backoff.SafeGatewayPlugininstead of bareGatewayPluginto maintain the crash guard.Root cause in Carbon
This is an upstream issue in
@buape/carbon, but the fix is applied on the OpenClaw side to avoid depending on an upstream release.Testing
pnpm buildpassespnpm checkpasses (format + lint + typecheck)provider.proxy.test.tstests still pass (one assertion updated:toBe(GatewayPlugin.prototype)→toBeInstanceOf(GatewayPlugin))Files changed
src/discord/monitor/gateway-plugin.tssrc/discord/monitor/gateway-plugin.crash-guard.test.tssrc/discord/monitor/provider.proxy.test.ts