fix(discord): distinguish 30032 cap error from generic sync failures - #48087
fix(discord): distinguish 30032 cap error from generic sync failures#48087itsXactlY wants to merge 1 commit into
Conversation
On loaded installs, the inner _safe_sync_slash_commands can hit
Discord's hard 100-global-application-commands cap (HTTP 400 / code
30032). Today this surfaces in the gateway log as:
[Discord] Slash command sync failed: 400 Bad Request (error code: 30032):
Maximum number of application commands reached (100).
…followed by a full stack trace — identical to every other sync
failure (timeouts, network errors, random HTTP 500s). The actual
cause is buried under noise that looks like a real bug.
This is especially bad because:
- It can trigger AFTER _record_command_sync_attempt already wrote a
'we tried' entry, with no matching success / rate-limit record.
- The operator has no hint that the cap was the actual cause or how
to fix it (drop plugins / trim COMMAND_REGISTRY).
Detect the cap error in the inner except of _run_post_connect_initialization
(where _safe_sync_slash_commands is awaited) BEFORE the rate-limit
fallback. Emit a distinct, actionable warning naming the cause and the
remediation. Never re-raises, so the outer 'sync failed' defensive
except never sees it.
Detection mirrors _is_discord_rate_limit:
- prefers exc.code == 30032 (set by discord.py's HTTPException)
- falls back to status == 400 + 'Maximum number of application
commands reached' in exc.text so older discord.py forks / mocks /
exotic transports are still covered.
Diff: 126 insertions, 0 deletions, 2 files, 1 commit.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused diagnostic improvement. Current main still routes a non-rate-limit exception from _safe_sync_slash_commands() through the generic traceback logger (plugins/platforms/discord/adapter.py:1705-1708,1745), so the premise remains valid.
Problems
plugins/platforms/discord/adapter.py:1315labels 30032 asHTTP 30032. The PR's helper docstring at:1245correctly describes this condition as HTTP 400 / error code 30032. The warning should preserve that distinction so the new diagnostic is accurate.
Suggested changes
- Log
HTTP 400 / error code 30032(orDiscord error code 30032) and assert that wording intests/gateway/test_discord_connect.py.
This is an automated hermes-sweeper review.
| # except or a misrouted rate-limit cooldown. | ||
| if self._is_discord_command_cap_error(e): | ||
| logger.warning( | ||
| "[%s] Discord slash command cap reached (HTTP 30032); " |
There was a problem hiding this comment.
30032 is Discord's application error code, not the HTTP status; the detector's docstring above correctly describes this as HTTP 400 / error code 30032. Please correct the warning so the new actionable diagnostic is itself accurate.
|
Good catch on the wording — fixed, and the whole change is rebuilt on current This branch is ~7.8k commits behind and conflicting, so rebasing it isn't meaningful; #75696 reimplements it against
7 tests in Happy to close this one in favour of #75696. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the same diagnostic gap: #48087 introduces narrow handling for Discord error code 30032, while #75696 rebuilds that handling on current main and corrects the warning to distinguish HTTP 400 from Discord error code 30032.
Related pull requests
- #48087
related— (+126/-0) — close as duplicate of #75696: #48087 adds a cap-error discriminator, actionable warning, and focused tests, but its warning incorrectly labels 30032 as an HTTP status. Despite the keep_open review on #48087, #75696 carries the same core fix on current main, addresses that review finding, and adds broader regression coverage. - #75696
duplicate— (+185/-0) — keep open with a salvage path: retain the narrow 30032 discriminator, the explicit “HTTP 400 / error code 30032” diagnostic, and tests proving unrelated failures still reach the generic handler. This agrees with the maintainer-bot keep_open verdict; unlike #48087, the diff also handles JSON data payloads and follows the current exception shape and surrounding implementation.
Duplicates
#48087 and #75696 implement substantially the same dedicated Discord command-cap diagnostic; #75696 is the current-main replacement that incorporates the review correction, so #48087 can be closed as a duplicate of #75696.
Suggested consolidation
Keep #75696 open with the salvage path identified by the maintainer-bot review: preserve its narrow error-code detection, actionable warning, and positive and negative regression tests. Close #48087 as a duplicate of #75696 because the latter reimplements the same fix on current main while correcting #48087’s HTTP-status wording and expanding compatibility and false-positive coverage.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup48087 ["PRs duplicating each other"]
P48087["PR #48087 (open)"]
P75696["PR #75696 (open)"]
end
class P48087 open
class P75696 open
class P48087 target
click P48087 "https://github.com/NousResearch/hermes-agent/pull/48087"
click P75696 "https://github.com/NousResearch/hermes-agent/pull/75696"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 18 kB of PR diffs, 6 kB of issue/PR text, 3 kB of discussion (3 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Problem
On loaded installs, the inner
_safe_sync_slash_commandscan hit Discord's hard 100-global-application-commands cap (HTTP 400 / code 30032). Today this surfaces in the gateway log as:…followed by a full stack trace — identical to every other sync failure (timeouts, network errors, random HTTP 500s). The actual cause is buried under noise that looks like a real bug.
This is especially bad because:
_record_command_sync_attemptalready wrote a "we tried" entry, with no matching success / rate-limit record.except Exceptionswallows it so the gateway survives, but the log is misleading.The pre-flight registration-time cap (
_DISCORD_MAX_APP_COMMANDS = 100at line 39) protects against hitting 30032 on registration, but doesn't make the sync resilient if 30032 surfaces for any reason (fetch/upsert drift, external command registration, plugin race conditions).Fix
Detect the cap error in the inner
exceptof_run_post_connect_initialization(where_safe_sync_slash_commandsis awaited) BEFORE the rate-limit fallback. Emit a distinct, actionable warning naming the cause and the remediation. Never re-raises, so the outer defensiveexceptnever sees it.Detection mirrors
_is_discord_rate_limit:exc.code == 30032(set by discord.py's HTTPException)status == 400+"Maximum number of application commands reached"inexc.textso older discord.py forks / mocks / exotic transports are still covered.Diff
126 lines added, 0 deleted, 1 commit. No churn.
Tests
test_post_connect_initialization_logs_cap_error_with_distinct_message"cap reached"log line, does NOT re-raise, does NOT fall through to the outer"sync failed"branchtest_is_discord_command_cap_error_detects_30032What this does NOT do
_DISCORD_MAX_APP_COMMANDS = 100) — that already exists._safe_sync_slash_commands— the existing diff-based sync logic is preserved.It's a pure logging-clarity change in the inner
exceptof_run_post_connect_initialization, plus a focused unit test for the discriminator helper.