Skip to content

docs: polish README and fix stale rest.go retry comment#6

Merged
HiLleywyn merged 1 commit into
mainfrom
claude/fix-carlos-bot-logging-MqMf8
Apr 13, 2026
Merged

docs: polish README and fix stale rest.go retry comment#6
HiLleywyn merged 1 commit into
mainfrom
claude/fix-carlos-bot-logging-MqMf8

Conversation

@HiLleywyn
Copy link
Copy Markdown
Owner

Summary

Documentation-only polish pass. No behaviour changes.

  • README.md

    • Opening paragraph now calls out the zero-dependency WebSocket client and the panic-recovered event dispatcher
    • New "Vendored builds" subsection under Installation documenting the GOWORK=off go mod vendor workflow for self-contained deployments
    • Rate Limiting section now names maxRateLimitRetries explicitly and notes that the retry budget is per-request
    • Security section extended to mention maxFramePayload (including the 8-byte negative-length rejection) and the url.PathEscape reaction path-injection fix
    • Troubleshooting note that 4014 close codes surface in the gateway log so they're easy to spot
  • rest.go

    • The file header comment said 429 responses were "retried once" — the actual behaviour is bounded by maxRateLimitRetries (3). Comment now matches the implementation.
  • CHANGELOG.md

    • Unreleased section added describing the README and rest.go header polish

Test plan

  • GOWORK=off go build ./...
  • go vet ./...
  • go test -race ./...
  • Verified all README claims match the code (maxRateLimitRetries, maxFramePayload, url.PathEscape in reaction methods)

https://claude.ai/code/session_01SJrLkkqYgfQnGi82DLeqvs

- README.md: clarified the opening paragraph to call out the
  zero-dependency WebSocket client and panic-recovered dispatcher,
  added a "Vendored builds" subsection under Installation with the
  GOWORK=off mod vendor workflow, named maxRateLimitRetries
  explicitly in the Rate Limiting section, extended the Security
  section with the maxFramePayload guard (including the negative
  payload-length rejection) and the url.PathEscape reaction fix, and
  added a note about the 4014 close code appearing in gateway logs.

- rest.go: the file header said 429 responses were "retried once";
  the actual behaviour is bounded by maxRateLimitRetries (3). Header
  comment now matches the implementation.

- CHANGELOG.md: added an Unreleased section describing the README
  and rest.go header-comment polish.
Copilot AI review requested due to automatic review settings April 13, 2026 21:55
@HiLleywyn HiLleywyn merged commit a70c254 into main Apr 13, 2026
8 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Documentation polish pass aligning user-facing docs with current library behavior and security posture, plus a small comment fix in the REST client header.

Changes:

  • Updated rest.go header comment to describe 429 retry behavior in terms of maxRateLimitRetries.
  • Expanded README.md with vendoring guidance, clearer rate-limit retry notes, and additional security/troubleshooting details.
  • Added an Unreleased entry in CHANGELOG.md describing the documentation/comment updates.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
rest.go Updates the file header comment for 429 retry behavior to match implementation.
README.md Adds/extends documentation for installation vendoring, rate limiting, security notes, and troubleshooting.
CHANGELOG.md Documents the above doc/comment changes under Unreleased.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md
Comment on lines +72 to 83
For self-contained deployments (e.g. scratch Docker images), vendor
the module and build with `-mod=vendor`:

```bash
GOWORK=off go mod tidy
GOWORK=off go mod vendor
go build -mod=vendor ./...
```

The `GOWORK=off` disables workspace mode so a sibling `go.work` file
doesn't pull in live-dev paths during vendoring.

Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new “Vendored builds” subsection duplicates the later “Vendoring” section (both show the same GOWORK=off … go mod vendor workflow). To avoid the two sections drifting over time, consider consolidating into a single section or having one link to the other.

Suggested change
For self-contained deployments (e.g. scratch Docker images), vendor
the module and build with `-mod=vendor`:
```bash
GOWORK=off go mod tidy
GOWORK=off go mod vendor
go build -mod=vendor ./...
```
The `GOWORK=off` disables workspace mode so a sibling `go.work` file
doesn't pull in live-dev paths during vendoring.
For self-contained deployments (e.g. scratch Docker images), see
[Vendoring](#vendoring) for the recommended `go mod vendor` workflow
and `-mod=vendor` build instructions.

Copilot uses AI. Check for mistakes.
Comment thread README.md
for the indicated duration, and retries the request. Retries are capped at
**3 attempts per call**; if the budget is exhausted a `*APIError` with
`StatusCode == 429` is returned so you can decide how to proceed.
**3 attempts per call** (`maxRateLimitRetries`); if the budget is exhausted a
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README says retries are capped at “3 attempts per call”, but the implementation allows up to 3 retries after the initial request (i.e. up to 4 total HTTP attempts). Consider rewording this to “up to 3 retries per call” (or “up to 4 attempts: 1 initial + 3 retries”) to match maxRateLimitRetries semantics in rest.go.

Suggested change
**3 attempts per call** (`maxRateLimitRetries`); if the budget is exhausted a
**up to 3 retries per call** (`maxRateLimitRetries`), for **up to 4 total
attempts** (1 initial request + 3 retries); if the budget is exhausted a

Copilot uses AI. Check for mistakes.
Comment thread README.md
Comment on lines +378 to +379
intents your bot requests. GoDiscord surfaces the 4014 close code in the
gateway log so it's straightforward to recognise in traces.
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This claims “GoDiscord surfaces the 4014 close code in the gateway log”, but the current WebSocket client treats close frames as io.EOF without parsing/logging the close code, and the gateway logs only a generic disconnect message. Either adjust the statement to reflect current logging, or update the gateway/websocket close handling to record the close code and reason.

Suggested change
intents your bot requests. GoDiscord surfaces the 4014 close code in the
gateway log so it's straightforward to recognise in traces.
intents your bot requests. If the bot connects and then immediately
disconnects after requesting one of these intents, verify that the matching
privileged intent is enabled in the portal.

Copilot uses AI. Check for mistakes.
@HiLleywyn HiLleywyn deleted the claude/fix-carlos-bot-logging-MqMf8 branch April 14, 2026 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants