feat: PR-3 add Discord API layer with rate limiting and pagination#3
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded Discord REST client with rate-limit and retry handling, Zod schemas for Discord payloads and search params, and paginated guild message search orchestration with progress/page callbacks; also added Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SearchAll as searchAllMessages
participant SearchOne as searchMessages
participant RateLimit as RateLimitMgr
participant DiscordFetch as discordFetch
participant DiscordAPI as Discord API
Client->>SearchAll: start(params, token, callbacks)
SearchAll->>SearchOne: request page(offset, maxId)
SearchOne->>RateLimit: waitForRateLimit(path, token)
RateLimit-->>SearchOne: permit / delay
SearchOne->>DiscordFetch: call(path, schema, token)
DiscordFetch->>DiscordAPI: HTTP request (Authorization + timeout)
DiscordAPI-->>DiscordFetch: response (status, headers, body)
alt 429 Too Many Requests
DiscordFetch->>RateLimit: update bucket from headers
DiscordFetch->>DiscordFetch: backoff + jitter, retry up to maxRetries429
else 202 Accepted
DiscordFetch->>DiscordFetch: parse retry_after, wait, retry up to maxRetries202
else 2xx OK
DiscordFetch->>DiscordFetch: parse JSON & validate with Zod
DiscordFetch-->>SearchOne: Ok(SearchResponse)
else other error
DiscordFetch-->>SearchOne: Err(DiscordFetchError)
end
SearchOne-->>SearchAll: page result
SearchAll->>Client: onPage / onProgress (progress updates)
loop pages/partitions
SearchAll->>SearchOne: next page (advance offset/maxId)
end
SearchAll-->>Client: accumulated messages or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/discord/search.ts">
<violation number="1" location="src/discord/search.ts:130">
P1: `searchAllMessages` can terminate after one page when `params.offset > 0` because `totalResults` is never initialized for non-zero starting offsets.</violation>
</file>
<file name="src/discord/client.ts">
<violation number="1" location="src/discord/client.ts:150">
P1: Error responses during 202 polling are not handled correctly. If a retry request returns 429 or other error status, it's passed to `parseResponse` which expects a success body, causing a confusing `ValidationError` instead of proper error handling. Check `!retryResponse.ok` before calling `parseResponse`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
mynameistito
changed the title
There was a problem hiding this comment.
2 issues found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/paths.ts">
<violation number="1" location="src/paths.ts:15">
P2: Handle only ENOENT here and create `.gitignore` atomically; broad catch + default write mode can overwrite unexpectedly.</violation>
</file>
<file name="src/discord/client.ts">
<violation number="1" location="src/discord/client.ts:169">
P2: 429 retries inside the 202 polling loop never reach the exhaustion branch because a 0-based attempt index is passed to `handle429`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/config.ts`:
- Around line 21-33: readSettingsFile currently calls JSON.parse(text) which
will throw on malformed JSON and leak to callers (affecting loadConfig and
saveSettings); update readSettingsFile to catch JSON parse errors around
JSON.parse(text) (or wrap the parse + schema validation in a try/catch) and
return {} when parsing fails so callers like loadConfig and saveSettings get a
consistent empty settings result rather than an exception; reference the
readSettingsFile function and ensure behavior aligns with
SettingsJsonSchema.safeParse usage so malformed JSON yields {} instead of
throwing.
In `@src/discord/client.ts`:
- Around line 154-156: The loop currently reuses the initial retryAfter for
every 202 retry; update the retry loop in the function containing the for (let
attempt = 0; attempt < maxRetries; attempt++) { ... } block to re-fetch and
re-parse the 202 response on each iteration, extract any updated retry_after
value and update the retryAfter used for the next sleep call (and/or call
waitForRateLimit with the new value) before awaiting sleep; also stop retrying
if the re-parsed response is no longer a 202. Reference: retryAfter,
waitForRateLimit, and the for (attempt...) retry loop.
- Around line 166-176: The 429 handling inside the 202 polling loop uses the
loop's `attempt` counter, conflating 202-poll retries with the 429 retry budget;
introduce a dedicated 429 retry counter (e.g., `attempt429` or `retry429`) local
to the 202 polling context and pass that to `handle429` instead of the outer
`attempt`, incrementing/resetting it only when a 429 is encountered and
respecting the same `maxRetries` logic inside this context; update calls and any
branching that relies on the returned `rateLimitResult` from `handle429` so the
outer 202 `attempt` remains unchanged by 429 events.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 399cc65e-5328-419f-8bca-93b89eea0cdb
📒 Files selected for processing (8)
.gitignoresrc/config.tssrc/discord/client.tssrc/discord/schemas.tssrc/discord/search.tssrc/errors.tssrc/paths.tssrc/types.ts
mynameistito
force-pushed
the
init/03-discord-api
branch
from
6df786e to
528f771
Compare
Add Zod schemas for Discord API contracts, HTTP client with automatic rate limit handling and exponential backoff, and paginated search with snowflake-based partitioning.
mynameistito
force-pushed
the
init/03-discord-api
branch
from
b0201a5 to
cea1f9b
Compare
|
@greptile |
Greptile SummaryThis PR introduces a typed Discord API layer with automatic rate limiting, retry logic, and snowflake-partitioned pagination for large-scale message fetching. The architecture is clean and the use of Key changes:
Confidence Score: 3/5Two P1 logic bugs must be fixed before merge: 202 polling mishandles error responses, and non-zero starting offsets silently break snowflake partitioning. The overall design is solid and the schemas are correct, but both client.ts and search.ts have correctness bugs that would cause incorrect runtime behavior in plausible real-world usage (rate-limited polling, resumable/offset-started fetches). Neither is a security issue, but they would silently produce wrong results or misleading errors. src/discord/client.ts (202 polling error handling) and src/discord/search.ts (totalResults initialisation guard) Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant discordFetch
participant handle429
participant handle202
participant Discord API
Caller->>discordFetch: discordFetch(path, schema, token)
loop attempt 0..maxRetries429
discordFetch->>discordFetch: waitForRateLimit()
discordFetch->>Discord API: GET request
Discord API-->>discordFetch: response
alt status 429
discordFetch->>handle429: handle429(response, attempt, max)
handle429->>handle429: parse retry_after
alt attempt >= maxRetries
handle429-->>discordFetch: Err(RateLimitExhaustedError)
discordFetch-->>Caller: Err(RateLimitExhaustedError)
else
handle429->>handle429: sleep(retry_after * 1000)
handle429-->>discordFetch: Ok(retry)
end
else status 202 (index not ready)
discordFetch->>handle202: handle202(response, url, token, schema, maxRetries202)
loop attempt 0..maxRetries202
handle202->>handle202: sleep(retry_after * 1000)
handle202->>Discord API: GET request (retry)
Discord API-->>handle202: response
alt status != 202
Note over handle202: Bug: skips ok check, calls parseResponse for 429/5xx too
handle202->>handle202: parseResponse(response, schema)
handle202-->>discordFetch: Ok(data) or Err(ValidationError)
end
end
handle202-->>discordFetch: Err(IndexNotReadyError)
else not ok (4xx/5xx)
discordFetch->>discordFetch: handleErrorResponse(response)
discordFetch-->>Caller: Err(DiscordApiError)
else ok (200)
discordFetch->>discordFetch: parseResponse(response, schema)
discordFetch-->>Caller: Ok(data)
end
end
Reviews (1): Last reviewed commit: "fix: ZodError issues type and gitignore ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…gination - Route handle202 retries through full status decision tree (429, error, OK) and re-read retry_after from each 202 body instead of reusing initial delay - Add Zod validation to 429 response body and exponential backoff with jitter - Extract magic numbers into named constants (retries, delays, page size, offset) - Add AbortController timeout (30s) to fetchWithAuth for stalled connections - Constrain limit, offset, and slop to valid integer ranges in SearchParamsSchema - Define explicit ThreadSchema/MemberSchema replacing z.unknown() in search response - Replace spread accumulator with Array.prototype.push.apply - Change ProgressCallback to accept SearchProgress object
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/discord/client.ts`:
- Around line 187-196: The 429 handling inside the 202 polling loop is
incorrectly sharing the loop's `attempt` counter, causing rate-limit retries to
consume the 202 retry budget; change the logic so `handle429` uses its own
independent retry counter (e.g., `rateLimitAttempt` or `retries429`) and its own
limit (e.g., `max429Retries`) instead of `attempt`/`maxRetries`, update the call
site that passes `attempt` to `handle429` to pass the new counter, and ensure
the loop still `continue`s after handling 429 without decrementing or advancing
the 202 `attempt` variable (keep `handle429` return behavior the same).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4508fc45-2dd6-4c86-ac13-a8ac9f2ac8a9
📒 Files selected for processing (4)
.gitignoresrc/discord/client.tssrc/discord/schemas.tssrc/discord/search.ts
The 429 handler was sharing the 202 loop's attempt counter, causing rate-limit retries to consume the polling retry budget. Introduced an independent rateLimitAttempt counter with its own MAX_429_RETRIES limit.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/discord/client.ts">
<violation number="1" location="src/discord/client.ts:195">
P2: 429 handling inside the 202 retry loop now ignores the caller-configured maxRetries429 and always uses MAX_429_RETRIES, so retry limits are inconsistent depending on whether the 429 happens during the initial request or during 202 polling.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/discord/client.ts`:
- Line 12: Rename the two similarly named 429-retry constants to clearly reflect
their different uses: change DEFAULT_MAX_RETRIES_429 to something like
MAIN_LOOP_429_MAX_RETRIES (or DEFAULT_429_RETRIES_MAIN_LOOP) to indicate it
governs top-level request loop retries, and change MAX_429_RETRIES to something
like POLL_202_429_MAX_RETRIES (or POLLING_429_MAX_RETRIES) to indicate it only
applies during 202 polling; update all usages and any nearby comments in the
module (references: DEFAULT_MAX_RETRIES_429 and MAX_429_RETRIES) so the intent
is explicit and unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: df62a0ab-5108-4618-8aff-45dc10831f1e
📒 Files selected for processing (1)
src/discord/client.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/discord/client.ts`:
- Around line 161-164: The current return uses parsed.data.retry_after ||
DEFAULT_RETRY_DELAY_SECONDS which treats 0 as falsy; change it to use nullish
coalescing so a server-provided 0 is respected: after
IndexNotReadyResponseSchema.safeParse(bodyResult.value) return parsed.success ?
parsed.data.retry_after ?? DEFAULT_RETRY_DELAY_SECONDS :
DEFAULT_RETRY_DELAY_SECONDS, referencing IndexNotReadyResponseSchema.safeParse,
parsed.success, parsed.data.retry_after and DEFAULT_RETRY_DELAY_SECONDS to
locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ac58cadc-afc2-4b0b-8d19-aeab20d3ced4
📒 Files selected for processing (1)
src/discord/client.ts
- Enforce strict snowflake validation for Discord IDs (guildId, authorId, channelId, etc.) - Parse SearchParamsSchema before search entrypoints to catch malformed input - Safely encode guildId in path segments to avoid API contract violations - Refactor rate-limit tracking to per-bucket state instead of global - Use bucket keys based on route family and token for independent throttling - Add response header bucket tracking for proper rate limit isolation
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/discord/client.ts">
<violation number="1" location="src/discord/client.ts:64">
P1: Bucket remapping logic orphans rate limit state. The locally computed `bucketKey` (`bucket:${hash}`) never matches the Discord-provided `X-RateLimit-Bucket` header (a hex string), so after remapping, subsequent requests can't find the stored state. Each request starts fresh with `remaining: 1`, defeating proactive rate limit tracking.
Consider storing a mapping from the computed key to the Discord bucket ID, or simply using the computed key consistently without remapping.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/discord/client.ts`:
- Around line 31-40: getBucketKey currently derives keys from full request path
plus token, but updateRateLimitState replaces that key with the raw
X-RateLimit-Bucket header (dropping token isolation) causing recreated buckets
and lost state; fix by standardizing on a composite bucket key that always
includes the canonical bucket id plus token (e.g., composeFrom(bucketId, token))
and update all call sites: change getBucketKey to return the composite form,
change updateRateLimitState to map header X-RateLimit-Bucket into that same
composite key (do not delete and reinsert under the raw header), and ensure
waitForRateLimit uses the identical composite key so state persists across
paginated requests and retries while remaining token-scoped.
In `@src/discord/search.ts`:
- Around line 161-165: The pagination stops early because the code only seeds
state.totalResults when offset === 0; change that condition to compare against
the actual start offset passed into searchAllMessages (queryParams.offset) so
initial pages that start at a non-zero offset still initialize totalResults.
Concretely, in fetchPage (and the other places flagged around lines 197-199 and
269) replace checks like "offset === 0 && !maxId" with "offset ===
queryParams.offset && !maxId" (or otherwise use the stored initialOffset from
searchAllMessages) so state.totalResults is set when the first request of this
run is made rather than only when offset is zero.
- Around line 41-52: The code in searchMessages and buildQueryString is
overwriting validated pagination (params.offset / params.maxId) with unvalidated
defaults/overrides (offset param default 0 and maxId arg), so validate and use
an effectiveOffset/effectiveMaxId derived from SearchParamsSchema-validated
values unless an explicit override is passed; when an override is provided,
validate that override against the same bounds/schema (e.g., offset range and
snowflake format) before calling buildQueryString, then pass the validated
effectiveOffset/effectiveMaxId into buildQueryString (and update the other call
sites that build the query string in the same file to follow the same pattern).
- Line 270: In searchAllMessages the code incorrectly uses
SearchParamsSchema.limit (queryParams.limit) as maxMessages which turns the
per-page Discord page-size into a hard total cap; change the logic so
queryParams.limit is treated only as the page-size (keep it as the per-request
limit used when fetching pages) and remove or replace the assignment
maxMessages: queryParams.limit; if you need a total-message ceiling introduce a
separate option (e.g., totalLimit or maxTotalMessages) and use that to stop the
partition loop, leaving maxMessages undefined or driven by the new totalLimit
instead of overloading queryParams.limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f1a8c20-83ad-4d67-b115-c8a5361450c2
📒 Files selected for processing (3)
src/discord/client.tssrc/discord/schemas.tssrc/discord/search.ts
…misuse - Fix bucket remapping: add bucketKeyMap so subsequent requests resolve computed keys to Discord-provided bucket IDs, preserving rate limit state - Fix totalResults init: use state.totalResults === 0 instead of offset === 0 so pagination works correctly when starting from non-zero offsets - Fix limit as maxMessages: searchAllMessages no longer caps total results at the per-page limit, allowing it to actually fetch all messages
|
@CodeRabbit Please do a deep analysis on this PR and see if there any issues that need to be resolved. bee as nit picky as possible. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 27695 🏁 Script executed: Length of output: 6122
🔴 Critical / High1. Rate-limit bucket key includes full query string (
|
| # | File | Severity | Issue |
|---|---|---|---|
| 1 | client.ts |
🔴 Critical | Bucket key hashes full URL incl. query string |
| 2 | client.ts |
🔴 High | TOCTOU: remaining never decremented optimistically |
| 3 | client.ts |
🔴 High | || instead of ?? for retry_after: 0 |
| 4 | client.ts |
🟠 Medium | Bucket always remapped on every response |
| 5 | schemas.ts |
🟠 Medium | snowflakeArraySchema duplicates inline schema |
| 6 | search.ts |
🟠 Medium | Silent sort override in searchAllMessages |
| 7 | search.ts |
🟠 Medium | maxMessages unreachable from callers |
| 8 | search.ts |
🟠 Medium | Redundant re-validation per page |
| 9 | client.ts |
🟠 Medium | Independent 429-counter inside 202 polling |
| 10 | client.ts |
🟡 Nit | RateLimitBodySchema not in schemas.ts |
| 11 | client.ts |
🟡 Nit | fetchWithAuth hardcoded to GET |
| 12 | schemas.ts |
🟡 Nit | Missing guild_id in MessageSchema |
| 13 | schemas.ts |
🟡 Nit | EmbedImageSchema/EmbedThumbnailSchema identical |
| 14 | search.ts |
🟡 Nit | DEFAULT_PAGE_SIZE is redundant alias of MAX_PAGE_SIZE |
| 15 | client.ts |
🟡 Nit | Hash can produce negative bucket key strings |
| 16 | — | 🟡 Nit | No unit tests |
🧠 Learnings used
Learnt from: CR
Repo: mynameistito/discord-search PR: 0
File: src/discord/AGENTS.md:0-0
Timestamp: 2026-03-26T03:41:36.696Z
Learning: Applies to src/discord/**/search.ts : Use Snowflake pagination (partition by snowflake) for results exceeding 10,000 items
Learnt from: CR
Repo: mynameistito/discord-search PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:26:40.141Z
Learning: Applies to src/discord/**/*.{ts,tsx} : Use Rate limiting with exponential backoff for Discord API interactions
Learnt from: CR
Repo: mynameistito/discord-search PR: 0
File: src/discord/AGENTS.md:0-0
Timestamp: 2026-03-26T03:41:36.696Z
Learning: Applies to src/discord/**/search.ts : Use BigInt for Snowflake parsing (cannot use Number type due to precision limits)
Learnt from: CR
Repo: mynameistito/discord-search PR: 0
File: src/discord/AGENTS.md:0-0
Timestamp: 2026-03-26T03:41:36.696Z
Learning: Applies to src/discord/**/*.ts : Do not skip Zod validation of API responses
Learnt from: CR
Repo: mynameistito/discord-search PR: 0
File: src/discord/AGENTS.md:0-0
Timestamp: 2026-03-26T03:41:36.696Z
Learning: Applies to src/discord/**/search.ts : Use offset pagination with 0-9975 range (25 results per page, max 400 pages)
… search Strip query string from rate limit bucket keys, optimistically decrement remaining count, use ?? for retry_after, fix redundant bucket remapping, share 429 retry counter between outer loop and 202 polling, deduplicate schemas, guard sort param overrides, expose maxMessages, eliminate double-validation per page, and minor cleanups.
1 participant
Summary
Stacked PR Chain
This is PR 3 of 7 — merge in order.
main← PR1 ← PR2 ← PR3 ← PR4 ← PR5 ← PR6 ← PR7Test plan
bun run typecheckpassesbun run checkpasses lintingSummary by cubic
Adds a typed Discord API layer with rate limiting, retries, and paginated search. Enables reliable large-scale message fetching with automatic backoff and schema validation.
zodschemas for users, messages, embeds, search responses, and params.discordFetchclient with rate-limit tracking, 429 retry with backoff, and 202 “index not ready” polling.searchMessagesandsearchAllMessageswith snowflake-based partitioning, offset/maxId support, and optional progress/page callbacks; caps page size to 25.better-resultwith typed errors:DiscordApiError,RateLimitExhaustedError,IndexNotReadyError,ValidationError.Written for commit 528f771. Summary will update on new commits.