Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/ripe-pandas-eat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"@knocklabs/client": minor
---

Add `maxRetries` option to `ApiRequestConfig`

When making a request to the Knock API using `ApiClient`'s `makeRequest` function, the `maxRetries` option of `ApiRequestConfig` can now be used to set the maximum number of times the request should be retried when a retryable error occurs. To disable retries altogether, set `maxRetries` to 0.

```js
const result = await knock.client().makeRequest({
method: "GET",
url: "/v1/users",
maxRetries: 0,
});
```
12 changes: 10 additions & 2 deletions packages/client/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ const STABLE_CONNECTION_THRESHOLD_MS = 30_000;
// counting.
const MAX_UNSTABLE_CONNECTION_COUNT = 15;

// The max number of times a request is retried after a retryable failure,
// unless overridden by `ApiRequestConfig.maxRetries`.
const DEFAULT_MAX_REQUEST_RETRIES = 3;

type ApiClientOptions = {
host: string;
apiKey: string;
Expand All @@ -44,6 +48,8 @@ export type ApiRequestConfig = {
data?: unknown;
headers?: HeadersInit;
signal?: AbortSignal;
/** Max number of retries for retryable failures. Set this to 0 to disable retries. */
maxRetries?: number;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be named maxRequestRetries to be consistent? also opens up for any other retries that may exist in the future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a bit redundant, though? This type will only be used with the API client’s makeRequest method:

knock.client().makeRequest({
  method: "GET",
  url: "/v1/users",
  // Redundant? Since we’re already using makeRequest here.
  maxRequestRetries: 0,
});

};

type FetchClient = (
Expand Down Expand Up @@ -249,9 +255,11 @@ class ApiClient {
private async requestWithRetries(req: ApiRequestConfig) {
let lastError: unknown;

const { maxRetries = DEFAULT_MAX_REQUEST_RETRIES } = req;

// Sequential retry loop: each attempt awaits in order and returns early on
// success or a non-retryable error, so it doesn't reduce to an array method.
for (let attempt = 0; attempt <= 3; attempt++) {
for (let attempt = 0; attempt <= maxRetries; attempt++) {
let retryAfterMs = 0;

try {
Expand Down Expand Up @@ -279,7 +287,7 @@ class ApiClient {
}
}

if (attempt < 3) {
if (attempt < maxRetries) {
await this.delay(this.getRetryDelay(attempt + 1, retryAfterMs));
}
}
Expand Down
69 changes: 69 additions & 0 deletions packages/client/test/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,75 @@ describe("API Client", () => {
expect(response.statusCode).toBe("error");
expect(fetchMock).toHaveBeenCalledTimes(1);
});

test("does not retry when maxRetries is 0", async () => {
const apiClient = new ApiClient({
host: "https://api.knock.app",
apiKey: "pk_test_12345",
userToken: undefined,
});
const fetchMock = vi
.fn()
.mockResolvedValueOnce(
createJsonResponse({ error: "Server Error" }, 500),
)
.mockResolvedValue(createJsonResponse({ success: true }));
setFetchMock(apiClient, fetchMock);
skipRetryDelays(apiClient);

const consoleSpy = vi
.spyOn(console, "error")
.mockImplementation(() => { });
try {
const response = await apiClient.makeRequest({
method: "GET",
url: "/test",
maxRetries: 0,
});

expect(response.statusCode).toBe("error");
expect(response.status).toBe(500);
expect(fetchMock).toHaveBeenCalledTimes(1);
} finally {
consoleSpy.mockRestore();
}
});

test("retries up to maxRetries times when overridden", async () => {
const apiClient = new ApiClient({
host: "https://api.knock.app",
apiKey: "pk_test_12345",
userToken: undefined,
});
// Return a fresh Response per call, like a real fetch — a Response body
// can only be consumed once, so each retry must get its own.
const fetchMock = vi
.fn()
.mockImplementation(() =>
createJsonResponse({ error: "Server Error" }, 500),
);
setFetchMock(apiClient, fetchMock);
skipRetryDelays(apiClient);

const consoleSpy = vi
.spyOn(console, "error")
.mockImplementation(() => { });
try {
const response = await apiClient.makeRequest({
method: "GET",
url: "/test",
maxRetries: 1,
});

expect(response.statusCode).toBe("error");
expect(response.status).toBe(500);

// The initial attempt plus one retry
expect(fetchMock).toHaveBeenCalledTimes(2);
} finally {
consoleSpy.mockRestore();
}
});
});

describe("Request Configuration", () => {
Expand Down
Loading