Skip to content

Commit 3e71a90

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

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
@@ -5,11 +5,12 @@ declare const __VERSION__: string
55
const DEFAULT_BASE_URL = "https://api.opensea.io"
66
const DEFAULT_TIMEOUT_MS = 30_000
77
const USER_AGENT = `opensea-cli/${__VERSION__}`
8-
const DEFAULT_MAX_RETRIES = 3
8+
const DEFAULT_MAX_RETRIES = 0
99
const DEFAULT_RETRY_BASE_DELAY_MS = 1_000
1010

11-
function isRetryableStatus(status: number): boolean {
12-
return status === 429 || status >= 500
11+
function isRetryableStatus(status: number, method: string): boolean {
12+
if (status === 429) return true
13+
return status >= 500 && method === "GET"
1314
}
1415

1516
function parseRetryAfter(header: string | null): number | undefined {
@@ -145,7 +146,11 @@ export class OpenSeaClient {
145146
return response
146147
}
147148

148-
if (attempt < this.maxRetries && isRetryableStatus(response.status)) {
149+
const method = init.method ?? "GET"
150+
if (
151+
attempt < this.maxRetries &&
152+
isRetryableStatus(response.status, method)
153+
) {
149154
const retryAfterMs = parseRetryAfter(
150155
response.headers.get("Retry-After"),
151156
)
@@ -159,7 +164,11 @@ export class OpenSeaClient {
159164
)
160165
}
161166

162-
await response.body?.cancel()
167+
try {
168+
await response.body?.cancel()
169+
} catch {
170+
// Stream may already be disturbed
171+
}
163172
await sleep(delayMs)
164173
continue
165174
}

test/client.test.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,14 +267,14 @@ describe("OpenSeaClient", () => {
267267
expect(fetch).toHaveBeenCalledTimes(2)
268268
})
269269

270-
it("retries post requests", async () => {
270+
it("retries post requests on 429", async () => {
271271
const retryClient = new OpenSeaClient({
272272
apiKey: "test-key",
273273
maxRetries: 3,
274274
retryBaseDelay: 100,
275275
})
276276
vi.spyOn(globalThis, "fetch")
277-
.mockResolvedValueOnce(new Response("Server Error", { status: 503 }))
277+
.mockResolvedValueOnce(new Response("Rate limited", { status: 429 }))
278278
.mockResolvedValueOnce(
279279
new Response(JSON.stringify({ status: "ok" }), {
280280
status: 200,
@@ -289,6 +289,22 @@ describe("OpenSeaClient", () => {
289289
expect(fetch).toHaveBeenCalledTimes(2)
290290
})
291291

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

0 commit comments

Comments
 (0)