Skip to content

Commit 7a79571

Browse files
committed
fix: do not retry POST requests on network errors by default
axios-retry's default retryCondition, isNetworkOrIdempotentRequestError, retries any network-level failure (timeout, connection reset) regardless of HTTP method. Since the SDK's write endpoints are POST requests, a connection reset or timeout mid-flight caused the same POST to be resent up to 3 times even though the server may have already received and acted on it, e.g., replaying an unlockDoor call. Restrict the default retryCondition to axios-retry's isIdempotentRequestError so only GET/HEAD/OPTIONS/PUT/DELETE are retried, matching the retry semantics already documented for the Ruby and Python SDKs. Callers can still opt back in per-request via axiosRetryOptions. Also corrects the README, which described timeout retries without noting they were previously replayed regardless of method.
1 parent dfc33d6 commit 7a79571

3 files changed

Lines changed: 88 additions & 1 deletion

File tree

‎README.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,17 @@ The Axios client and retry behavior may be configured with custom initiation opt
474474
via [`axiosOptions`][axiosOptions] and [`axiosRetryOptions`][axiosRetryOptions].
475475
Options are deep merged with the default options.
476476

477+
By default, up to 2 retries are attempted for a failure that is either
478+
a retryable status code (`429` or `5xx`) or a network-level error,
479+
e.g., a timeout or a connection reset,
480+
but only for HTTP methods considered idempotent (`GET`, `HEAD`, `OPTIONS`, `PUT`, `DELETE`).
481+
`POST` requests, e.g., most write operations against the Seam API,
482+
are not retried by default:
483+
since the client cannot tell whether the server received and processed
484+
the request before the connection was lost,
485+
automatically resending it could repeat a side effect, such as unlocking a door twice.
486+
Pass a custom `retryCondition` in `axiosRetryOptions` to change this behavior.
487+
477488
[axiosOptions]: https://axios-http.com/docs/config_defaults
478489
[axiosRetryOptions]: https://github.com/softonic/axios-retry
479490

‎src/lib/client.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import axios, { type AxiosInstance, type AxiosRequestConfig } from 'axios'
2-
import axiosRetry, { type AxiosRetry, exponentialDelay } from 'axios-retry'
2+
import axiosRetry, {
3+
type AxiosRetry,
4+
exponentialDelay,
5+
isIdempotentRequestError,
6+
} from 'axios-retry'
37

48
import { errorInterceptor } from './error-interceptor.js'
59
import { serializeUrlSearchParams } from './url-search-params-serializer.js'
@@ -27,6 +31,13 @@ export const createClient = (options: ClientOptions): AxiosInstance => {
2731
axiosRetry(client, {
2832
retries: 2,
2933
retryDelay: exponentialDelay,
34+
// axios-retry's own default, isNetworkOrIdempotentRequestError, retries
35+
// network errors (e.g., a connection reset or a timeout) regardless of
36+
// HTTP method. That resends the body of an in-flight, non-idempotent
37+
// request (e.g., a POST) whenever the client never saw a response,
38+
// even though the server may have already received and processed it.
39+
// Restrict retries to methods that are safe to send more than once.
40+
retryCondition: isIdempotentRequestError,
3041
...options.axiosRetryOptions,
3142
})
3243

‎test/seam/connect/retry.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { createServer } from 'node:http'
2+
13
import test from 'ava'
24
import { AxiosError } from 'axios'
35
import { getTestServer } from 'fixtures/seam/connect/api.js'
@@ -35,3 +37,66 @@ test('SeamHttp: retries 503 status errors twice by default ', async (t) => {
3537

3638
t.is(err?.response?.status, 503)
3739
})
40+
41+
test('SeamHttp: does not retry POST requests by default', async (t) => {
42+
const { seed, endpoint } = await getTestServer(t)
43+
44+
const seam = SeamHttp.fromApiKey(seed.seam_apikey1_token, {
45+
endpoint,
46+
axiosRetryOptions: {
47+
onRetry: () => {
48+
t.fail('should not retry a POST request')
49+
},
50+
},
51+
})
52+
53+
await seam.client.post('/_fake/simulate_workspace_outage', {
54+
workspace_id: seed.seed_workspace_1,
55+
routes: ['/devices/list'],
56+
})
57+
58+
// devices.list is a POST under the hood, so a mid-flight failure here must
59+
// not be replayed.
60+
const err = await t.throwsAsync(async () => await seam.devices.list(), {
61+
instanceOf: AxiosError,
62+
})
63+
64+
t.is(err?.response?.status, 503)
65+
})
66+
67+
test('SeamHttp: does not replay a POST after a mid-flight connection reset', async (t) => {
68+
let attempts = 0
69+
70+
// A raw server that receives the full request and then resets the
71+
// connection without ever sending a response, e.g., as if a load balancer
72+
// or proxy dropped the connection after forwarding the request upstream.
73+
const server = createServer((req) => {
74+
attempts++
75+
req.resume()
76+
req.on('end', () => {
77+
req.socket.destroy()
78+
})
79+
})
80+
81+
await new Promise<void>((resolve) => {
82+
server.listen(0, resolve)
83+
})
84+
t.teardown(async () => {
85+
await new Promise<void>((resolve) => {
86+
server.close(() => resolve())
87+
})
88+
})
89+
90+
const address = server.address()
91+
if (address == null || typeof address === 'string') {
92+
throw new Error('Could not determine server address')
93+
}
94+
95+
const seam = SeamHttp.fromApiKey('seam_invalidapikey_token', {
96+
endpoint: `http://127.0.0.1:${address.port}`,
97+
})
98+
99+
await t.throwsAsync(async () => await seam.devices.list())
100+
101+
t.is(attempts, 1)
102+
})

0 commit comments

Comments
 (0)