Skip to content

Commit 0e3538a

Browse files
fix: address code review feedback (POST idempotency, SDK default, stream cancel safety)
Co-Authored-By: Chris K <ckorhonen@gmail.com>
1 parent 93a665e commit 0e3538a

2 files changed

Lines changed: 32 additions & 7 deletions

File tree

src/client.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@ import type { OpenSeaClientConfig } from "./types/index.js"
22

33
const DEFAULT_BASE_URL = "https://api.opensea.io"
44
const DEFAULT_TIMEOUT_MS = 30_000
5-
const DEFAULT_MAX_RETRIES = 3
5+
const DEFAULT_MAX_RETRIES = 0
66
const DEFAULT_RETRY_BASE_DELAY_MS = 1_000
77

8-
function isRetryableStatus(status: number): boolean {
9-
return status === 429 || status >= 500
8+
function isRetryableStatus(status: number, method: string): boolean {
9+
if (status === 429) return true
10+
return status >= 500 && method === "GET"
1011
}
1112

1213
function parseRetryAfter(header: string | null): number | undefined {
@@ -135,7 +136,11 @@ export class OpenSeaClient {
135136
return response
136137
}
137138

138-
if (attempt < this.maxRetries && isRetryableStatus(response.status)) {
139+
const method = init.method ?? "GET"
140+
if (
141+
attempt < this.maxRetries &&
142+
isRetryableStatus(response.status, method)
143+
) {
139144
const retryAfterMs = parseRetryAfter(
140145
response.headers.get("Retry-After"),
141146
)
@@ -149,7 +154,11 @@ export class OpenSeaClient {
149154
)
150155
}
151156

152-
await response.body?.cancel()
157+
try {
158+
await response.body?.cancel()
159+
} catch {
160+
// Stream may already be disturbed
161+
}
153162
await sleep(delayMs)
154163
continue
155164
}

test/client.test.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,14 +264,14 @@ describe("OpenSeaClient", () => {
264264
expect(fetch).toHaveBeenCalledTimes(2)
265265
})
266266

267-
it("retries post requests", async () => {
267+
it("retries post requests on 429", async () => {
268268
const retryClient = new OpenSeaClient({
269269
apiKey: "test-key",
270270
maxRetries: 3,
271271
retryBaseDelay: 100,
272272
})
273273
vi.spyOn(globalThis, "fetch")
274-
.mockResolvedValueOnce(new Response("Server Error", { status: 503 }))
274+
.mockResolvedValueOnce(new Response("Rate limited", { status: 429 }))
275275
.mockResolvedValueOnce(
276276
new Response(JSON.stringify({ status: "ok" }), {
277277
status: 200,
@@ -286,6 +286,22 @@ describe("OpenSeaClient", () => {
286286
expect(fetch).toHaveBeenCalledTimes(2)
287287
})
288288

289+
it("does not retry post requests on 5xx", async () => {
290+
const retryClient = new OpenSeaClient({
291+
apiKey: "test-key",
292+
maxRetries: 3,
293+
retryBaseDelay: 100,
294+
})
295+
vi.spyOn(globalThis, "fetch").mockResolvedValue(
296+
new Response("Server Error", { status: 503 }),
297+
)
298+
299+
await expect(retryClient.post("/api/v2/refresh")).rejects.toThrow(
300+
OpenSeaAPIError,
301+
)
302+
expect(fetch).toHaveBeenCalledTimes(1)
303+
})
304+
289305
it("logs retries when verbose is enabled", async () => {
290306
const verboseRetryClient = new OpenSeaClient({
291307
apiKey: "test-key",

0 commit comments

Comments
 (0)