fix(discord): report the 30032 command cap as itself, not a generic sync failure - #75696
fix(discord): report the 30032 command cap as itself, not a generic sync failure#75696itsXactlY wants to merge 1 commit into
Conversation
…ync failure Rebuilds NousResearch#48087 on current main and applies its review feedback. Discord rejects the entire slash-command sync batch with HTTP 400 / error code 30032 ("Maximum number of application commands reached") once an app holds 100 global commands. That rejection is neither a 429 — so the rate-limit branch in _run_post_connect_initialization() skips it — nor recognizable in the outer handler, which logs it as "Slash command sync failed" with a stack trace. The operator is left with no hint that the fix is to free command slots, and every slash command stays broken. A narrow _is_discord_command_cap_error() now precedes the rate-limit check and logs the condition in plain words. Review feedback applied: the warning names HTTP 400 *and* error code 30032 separately. 30032 is a Discord JSON error code, not an HTTP status; labelling it "HTTP 30032" would send an operator looking in the wrong place. The test asserts both fragments. Detection follows the house style of _is_discord_unknown_interaction rather than the original PR's narrower version: it also reads the code out of a JSON `data` payload and prefers exc.status over exc.response.status. It stays deliberately narrow — a bare 400 proves nothing (50035 "Invalid Form Body" is also a 400), so a match needs the 30032 code, or a 400 plus the specific cap message for older forks and mocks that expose no code. Anything else keeps raising, so unrelated bugs still surface with their traceback. Tests (tests/gateway/test_discord_connect.py): code attr, JSON payload code, legacy message-only fallback, unrelated 400 ignored, 429 ignored, the end-to-end log wording, and an unrelated RuntimeError still reaching the generic handler.
|
Thanks for rebuilding the diagnostic on current main. The premise is confirmed: Main's existing safeguards are complementary rather than duplicative: it deletes obsolete commands before creates at Automated hermes-sweeper review. |
|
Thanks for the confirmation — noting the point about main's existing safeguards being complementary: the delete-before-create ordering and the registration bound both reduce how often a 30032 is reachable, but neither says anything when one still surfaces, which is exactly the gap this fills. No changes made in response; the PR stands as reviewed. |
SummaryTwo PRs address the same Discord command-cap diagnostic gap: #48087 adds narrow handling for error code 30032, while #75696 rebuilds the fix on current main, correctly distinguishes HTTP 400 from Discord error code 30032, and broadens positive and negative regression coverage. Related pull requests
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 consolidationKeep #75696 open with the salvage path confirmed by the maintainer-bot review: retain its narrow error-code detection, actionable HTTP 400 / error code 30032 diagnostic, 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 graphflowchart 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 P75696 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. |
Rebuilds #48087 against current
mainand applies the review feedback from that PR.#48087's head is ~7.8k commits behind and conflicting, so this reimplements the fix on
mainas it stands. The premise still holds there: a non-rate-limit exception out of_safe_sync_slash_commands()still falls through to the generic traceback logger atplugins/platforms/discord/adapter.py:1990.Review point, addressed
Fixed. 30032 is a Discord JSON error code, not an HTTP status — labelling it
HTTP 30032sends an operator looking in the wrong place. The warning now reads:and
test_cap_error_logs_http_400_and_error_codeasserts"HTTP 400"and"error code 30032"as separate fragments, so the distinction can't silently regress. The code is now a named constant,_DISCORD_COMMAND_CAP_ERROR_CODE, sitting next to the existing_DISCORD_MAX_APP_COMMANDS = 100.What changed from #48087
Detection follows the house style of the neighbouring
_is_discord_unknown_interaction()rather than #48087's version — it also reads the code out of a JSONdatapayload and prefersexc.statusoverexc.response.status, which is how discord.py actually shapesHTTPException.It stays deliberately narrow, mirroring
_is_discord_rate_limit(): a bare 400 proves nothing (50035 "Invalid Form Body" is also a 400), so a match requires the 30032 code, or a 400 plus the specific cap message for older forks, mocks and exotic transports that expose no code. Everything else keeps raising, so unrelated bugs still surface with their traceback — covered by a test.The cap check sits before the rate-limit branch because the cap is neither a 429 nor transient; routing it through a rate-limit cooldown would be wrong.
Tests
tests/gateway/test_discord_connect.py::TestDiscordCommandCapDiagnostic— 7 tests:codeattribute, code from a JSONdatapayload, legacy message-only fallback, unrelated 400 (50035) ignored, 429 and plain exceptions ignored, the end-to-end log wording (including that it does not reach the generic "Slash command sync failed" branch), and an unrelatedRuntimeErrorstill reaching that generic handler.All 7 pass. Full
-k discordrun overtests/gatewaycompared against untouchedmain: identical 4-failure set (pre-existing, unrelated —test_discord_send.pymock-moduleFileattribute,test_send_multiple_images.pyredirect), branch adds exactly the 7 new passes.Closes #48087.