fix: enrich RateLimitError message and its property

[nyannyacha]

What kind of change does this PR introduce?

Enhancement

Description

Add the number of milliseconds until the rate-limit window resets to the message and its property.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 5459ce2b-fcde-4fd8-a06f-6390558ac880

📥 Commits

Reviewing files that changed from the base of the PR and between da4ba85 and d12e47a.

📒 Files selected for processing (4)

• crates/base/tests/integration_tests.rs
• ext/node/polyfills/http.ts
• ext/runtime/js/errors.js
• ext/runtime/rate_limit.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• ext/node/polyfills/http.ts

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • RateLimitError now exposes retryAfterMs (milliseconds until reset) so clients can schedule retries precisely.
  • Rate-limit error messages include retry-after guidance to clarify when to retry.

• Improvements

  • More accurate outbound rate-limit detection and retry-after calculations for HTTP requests.

• Tests

  • Added integration test ensuring retryAfterMs is a positive, non-zero value when rate-limited.

Walkthrough

This change surfaces outbound rate-limit retry delays through the stack. The runtime op op_check_outbound_rate_limit now returns a u32 where 0xFFFFFFFF means allowed and any other value is retry-after milliseconds. Rate limiter methods return Ok(()) or Err(retry_after_ms). The JS/Fetch and polyfill layers construct and throw a RateLimitError(message, retryAfterMs). Type definitions and tests were added/updated to observe retryAfterMs.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant FetchLayer as Fetch/HTTP Layer
    participant RuntimeOp as Runtime Op
    participant RateLimiter as Rate Limiter
    participant ErrorCtor as Error Constructor

    Client->>FetchLayer: Start outbound request (url)
    FetchLayer->>RuntimeOp: op_check_outbound_rate_limit(url, key, is_traced)
    RuntimeOp->>RateLimiter: check_and_increment(...)
    
    alt Allowed (u32::MAX / 0xFFFFFFFF)
        RateLimiter-->>RuntimeOp: Ok(())
        RuntimeOp-->>FetchLayer: 0xFFFFFFFF
        FetchLayer->>FetchLayer: perform network request
        FetchLayer-->>Client: HTTP response
    else Denied (retry-after)
        RateLimiter-->>RuntimeOp: Err(retry_after_ms)
        RuntimeOp-->>FetchLayer: retry_after_ms (u32)
        FetchLayer->>ErrorCtor: new RateLimitError(msg, retryAfterMs)
        ErrorCtor-->>FetchLayer: RateLimitError instance
        FetchLayer-->>Client: throw RateLimitError (contains retryAfterMs)
    end

Loading

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

ext/runtime/rate_limit.rs (1)

191-196: ⚠️ Potential issue | 🔴 Critical

Don't encode a missing global_key as retryAfterMs = 0.

Line 195 feeds a configuration failure into the new retry-after channel. op_check_outbound_rate_limit() maps every Err(_) into a retry value, so the JS callers will surface this as RateLimitError.retryAfterMs === 0. Callers that honor the new property can read that as “retry immediately”, even though there is no rate-limit window to reset. Please keep this path distinct from real rate-limit denials, or make global_key mandatory before constructing TraceRateLimiter.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ext/runtime/rate_limit.rs` around lines 191 - 196, The current early-return
for missing global_key in TraceRateLimiter uses Err(0) which
op_check_outbound_rate_limit maps to retryAfterMs=0; instead return a distinct
configuration error (not zero) or make global_key mandatory before constructing
TraceRateLimiter so it never hits this branch: replace the return Err(0) at the
self.global_key.as_deref() check with a dedicated error value/variant (e.g.,
ConfigMissing or a non-zero errno) and update op_check_outbound_rate_limit to
treat that variant as a configuration failure (no retry) rather than a
rate-limit retry; alternatively enforce global_key presence at TraceRateLimiter
construction time so the code path never executes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@ext/runtime/rate_limit.rs`:
- Around line 191-196: The current early-return for missing global_key in
TraceRateLimiter uses Err(0) which op_check_outbound_rate_limit maps to
retryAfterMs=0; instead return a distinct configuration error (not zero) or make
global_key mandatory before constructing TraceRateLimiter so it never hits this
branch: replace the return Err(0) at the self.global_key.as_deref() check with a
dedicated error value/variant (e.g., ConfigMissing or a non-zero errno) and
update op_check_outbound_rate_limit to treat that variant as a configuration
failure (no retry) rather than a rate-limit retry; alternatively enforce
global_key presence at TraceRateLimiter construction time so the code path never
executes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 765f1c7e-3334-4b37-a1ee-c38187c4f4b9

📥 Commits

Reviewing files that changed from the base of the PR and between bdf1893 and da4ba85.

📒 Files selected for processing (8)

• crates/base/test_cases/rate-limit-retry-after/index.ts
• crates/base/tests/integration_tests.rs
• ext/node/polyfills/http.ts
• ext/runtime/js/errors.js
• ext/runtime/lib.rs
• ext/runtime/rate_limit.rs
• types/global.d.ts
• vendor/deno_fetch/26_fetch.js