feat: add APIError, rate limit parsing, and retry backoff #15

salemalem · 2025-11-14T22:08:47Z

No description provided.

salemalem · 2025-11-16T21:38:26Z

guys?

TheEdgeOfRage · 2025-11-20T16:10:14Z

Thank you for your PR 🙏 The whole team is currently out of office on a conference, so we will not be able to review this until next week. Sorry about the delay

salemalem · 2025-11-20T18:14:58Z

@TheEdgeOfRage which conference are you referring to? ETH DevCon Argentina?

TheEdgeOfRage · 2025-11-20T16:08:22Z

dune/execution.go

+			if maxRetries != 0 && errAttempts >= maxRetries {
 				return nil, fmt.Errorf("%w. %s", ErrorRetriesExhausted, err.Error())
 			}
-			fmt.Fprintln(os.Stderr, "failed to retrieve results. Retrying...\n", err)


Why remove the print here?

TheEdgeOfRage · 2025-12-05T12:01:13Z

dune/http.go

+	remStr := h.Get("X-RateLimit-Remaining")
+	resetStr := h.Get("X-RateLimit-Reset")
+
+	var lim, rem int


No need to shorten variable names :)

Suggested change

var lim, rem int

var limit, remaining int

TheEdgeOfRage · 2025-12-05T12:02:26Z

dune/http.go

+	InitialBackoff:       500 * time.Millisecond,
+	MaxBackoff:           5 * time.Second,


Given that rate limits apply on a minute basis, I think it makes sense to bump these a bit higher, as it's very possible that you'll still be rate limited after waiting only a couple of seconds

TheEdgeOfRage · 2025-12-05T12:04:10Z

dune/http.go

+	return &RateLimit{Limit: lim, Remaining: rem, Reset: reset}
+}
+
+func nextBackoff(attempt int, p RetryPolicy) time.Duration {


And would you mind moving the retry and backoff code into a separate file please (e.g. dune/retries.go) 🙏

Suggested change

func nextBackoff(attempt int, p RetryPolicy) time.Duration {

func (p RetryPolicy) nextBackoff(attempt int) time.Duration {

TheEdgeOfRage · 2025-12-05T12:04:55Z

dune/http.go

-		if err != nil {
-			return nil, fmt.Errorf("failed to read error response body: %w", err)
+		snippetBytes, _ := io.ReadAll(io.LimitReader(resp.Body, 1024))
+		var er ErrorResponse


Suggested change

var er ErrorResponse

var errorResp ErrorResponse

TheEdgeOfRage · 2025-12-05T12:05:30Z

dune/http.go

-			return nil, fmt.Errorf("failed to read error response body: %w", err)
+		snippetBytes, _ := io.ReadAll(io.LimitReader(resp.Body, 1024))
+		var er ErrorResponse
+		_ = json.Unmarshal(snippetBytes, &er)


This unmarshal could fail, since the body is capped at 1024 characters, so you'll need to handle the error here in that case

TheEdgeOfRage · 2025-12-05T12:06:32Z

@salemalem I'm sorry about this, I seem to have accidentally deleted the notification for this PR and forgot about it 😬

Yeah, it was DevConnect :)

salemalem · 2025-12-05T12:16:52Z

@TheEdgeOfRage ok let me review your suggestions and get back to you.

…; use method-based backoff

cursor · 2025-12-15T22:54:25Z

PR Summary

Adds a configurable retry policy with exponential backoff and enhances HTTP/error handling (APIError, rate limits, Retry-After), applying backoff in execution result polling.

HTTP:
- Introduces APIError and RateLimit with parseRateLimitHeaders to surface status, body snippet, and rate limit metadata.
- Reworks httpRequest to use defaultRetryPolicy for network errors and retryable statuses (429, 5xx), honoring Retry-After and returning detailed errors.
Execution:
- Updates WaitGetResults to track errAttempts, apply exponential backoff between retries, and reset attempts on success.
Retry Policy:
- Adds dune/retries.go defining RetryPolicy, defaultRetryPolicy, and NextBackoff (exponential with jitter).

^{Written by Cursor Bugbot for commit 7646e87. Configure here.}

salemalem · 2025-12-15T22:55:31Z

@TheEdgeOfRage thanks for the thorough review — I’ve addressed your points and aligned with house style:

Reintroduced stderr print in execution.WaitGetResults and switched to method-based backoff via defaultRetryPolicy.NextBackoff(...).
Renamed rate-limit variables to limit and remaining in parseRateLimitHeaders.
Moved retry/backoff into dune/retries.go and changed to func (p RetryPolicy) NextBackoff(int) time.Duration.
Bumped backoff windows to reflect minute-level limits: initialBackoff = 2s, maxBackoff = 60s, maxAttempts = 5 (with small jitter). Happy to tune these if you prefer different values.
Error parsing in httpRequest:
- Switched er → errorResp.
- Guarded json.Unmarshal and fall back to the raw 1KB body snippet if truncated or malformed.
- Return structured APIError with statusCode, statusText, bodySnippet, rateLimit, and retryAfter.
- Honor Retry-After when larger than computed backoff; retry for 429, 5xx and network errors.

I’ll resolve the current merge conflicts and push the updates. If you’d like a different jitter strategy or backoff defaults, I can adjust quickly. Thanks again!

cursor

Comment @cursor review or bugbot run to trigger another review on this PR

cursor · 2025-12-15T22:59:09Z

dune/http.go

+				}
+				time.Sleep(sleep)
+				attempt++
+				continue


Bug: POST request body empty on retry

When retrying a request after receiving a retryable status code (429, 5xx), the code reuses the same http.Request object without resetting its body. The request body (req.Body) is consumed by http.DefaultClient.Do() on the first attempt. On subsequent retry attempts, the body is empty because it has already been read. POST requests with JSON payloads (like QueryExecute, SQLExecute, QueryPipelineExecute) will send empty bodies on retry, causing silent failures. The fix would require calling req.GetBody() to regenerate the body before each retry.

Additional Locations (1)

dune/http.go#L146-L154

cursor · 2025-12-15T22:59:09Z

dune/http.go

+		}

-	if resp.StatusCode != 200 {
 		defer resp.Body.Close()


Bug: Defer in loop accumulates unclosed response bodies

The defer resp.Body.Close() statement is inside the retry loop. Each time the loop iterates with a non-200 status code, a new defer is added without the previous response body being closed immediately. This keeps multiple response bodies and their associated resources (TCP connections, file descriptors) open until the function returns, which could exhaust connection pool slots or cause "too many open files" errors during extended retry sequences.

…s; move RetryPolicy

feat: add APIError, rate limit parsing, and retry backoff

5f45e11

TheEdgeOfRage requested changes Dec 5, 2025

View reviewed changes

refactor: move RetryPolicy to dune/retries.go; rename rate-limit vars…

7646e87

…; use method-based backoff

cursor bot reviewed Dec 15, 2025

View reviewed changes

fix: reset POST body on retry; close resp bodies in loop; JSON header…

d4ed487

…s; move RetryPolicy

		InitialBackoff: 500 * time.Millisecond,
		MaxBackoff: 5 * time.Second,

	func nextBackoff(attempt int, p RetryPolicy) time.Duration {
	func (p RetryPolicy) nextBackoff(attempt int) time.Duration {

feat: add APIError, rate limit parsing, and retry backoff #15

Are you sure you want to change the base?

feat: add APIError, rate limit parsing, and retry backoff #15

Uh oh!

Conversation

salemalem commented Nov 14, 2025

Uh oh!

salemalem commented Nov 16, 2025

Uh oh!

TheEdgeOfRage commented Nov 20, 2025

Uh oh!

salemalem commented Nov 20, 2025

Uh oh!

TheEdgeOfRage Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

TheEdgeOfRage Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

TheEdgeOfRage Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

TheEdgeOfRage Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

TheEdgeOfRage Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

TheEdgeOfRage Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

TheEdgeOfRage commented Dec 5, 2025

Uh oh!

salemalem commented Dec 5, 2025

Uh oh!

cursor bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

salemalem commented Dec 15, 2025

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Dec 15, 2025

Choose a reason for hiding this comment

Bug: POST request body empty on retry

Uh oh!

cursor bot Dec 15, 2025

Choose a reason for hiding this comment

Bug: Defer in loop accumulates unclosed response bodies

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cursor bot commented Dec 15, 2025 •

edited

Loading