[vuanhphung]

Problem

On the Thrift backend a single transient HTTP error fails the connection or the statement, while the kernel (SEA) backend retries it:

• OpenSession is not in retryableThriftMethods, so a 503, 429, 502, 504 or 408 on session open is surfaced at once (1 attempt), and retriesTimeout has no effect on it.
• ExecuteStatement is retried only when the connection fails before the request is sent (fix: retry ExecuteStatement when the connection fails before the request is sent #598). A 429, or a 503 with Retry-After, means the server did not run the statement, but it is still surfaced.
• HttpRetryPolicy does not retry 408 on any request.

Found by the cross-driver error-recovery spec (ERRORRECOV-001, 007–012): live runs against main (7be705e) through the test proxy showed exactly 1 Thrift attempt in every case, while the SEA leg retried and passed.

Change

• Add OpenSession to the idempotent methods. A repeated open leaves at most an unused session, which the server expires.
• Retry 408 in HttpRetryPolicy (idempotent requests, CloudFetch and feature-flag fetches).
• ExecuteStatement now also retries 429 and 503 with Retry-After, the same rule as the kernel's non-idempotent strategy (src/client/retry.rs). 500, 502, 504, 408 and 503 without Retry-After are still not retried, so a statement can't run twice. UnsentRequestRetryPolicy is renamed NonIdempotentRetryPolicy.

All retries use the existing retryMaxAttempts / retriesTimeout budget.

Verification

• npm test (1764 passing), npm run lint, npm run type-check, npm run prettier.
• New unit tests cover OpenSession retry on 503/429/408, ExecuteStatement retry on 429 and 503+Retry-After, no retry on 503 without Retry-After, 500/502/504/408, and RetryError on exhaustion.
• With this branch built, the databricks-driver-test ERRORRECOV-001/007/008/009/010/011/012 Thrift tests (databricks/databricks-driver-test#2074) pass the spec assertions live (their xfail tripwires throw XPASS), and ERRORRECOV-013/014 (Reyden KP001 recovery) still pass.

Not changed here: getRetryAfterHeader compares the header's seconds with retryDelayMin in milliseconds, so a Retry-After below 1000 seconds is ignored and the plain exponential backoff applies.

This pull request and its description were written by Isaac.

[peco-review-bot]

Verdict: 1 Low

Looks good: one low concern about changelog wording. The retry classification is right: 429 and 503+Retry-After retry on ExecuteStatement, the ambiguous statuses don't, and OpenSession now uses the idempotent policy. Tests cover each case at the policy level and in the end-to-end connection tests. Follow-up work outside this PR: the existing Retry-After seconds-vs-milliseconds mismatch in getRetryAfterHeader, which the description already mentions.

[peco-review-bot]

✅ No issues identified by the review bot.