Skip to content

Commit bfe17e3

Browse files
hexedpacketshexedpackets
authored
Use retryable HTTP requests for Vault API (#1375)
## Description By default API requests will immediately fail. FGA endpoints are the exception that use a wrapper to retry some failures. This adds Vault paths to the same retry logic. It also adds `408` response codes to the list of retryable codes - `408` is set by the SDK when catching/rethrowing a [request timeout](https://github.com/workos/workos-node/blob/main/src/common/net/fetch-client.ts#L251). ## Documentation Does this require changes to the WorkOS Docs? E.g. the [API Reference](https://workos.com/docs/reference) or code snippets need updates. ``` [ ] Yes ``` If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.
1 parent e25424b commit bfe17e3

File tree

5 files changed

+129
-9
lines changed

5 files changed

+129
-9
lines changed

‎src/common/net/fetch-client.spec.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,62 @@ describe('Fetch client', () => {
8282
expect(await response.toJSON()).toEqual({ data: 'response' });
8383
});
8484

85+
it('get for Vault path should call fetchRequestWithRetry and return response', async () => {
86+
fetchOnce({ data: 'response' });
87+
const mockFetchRequestWithRetry = jest.spyOn(
88+
FetchHttpClient.prototype as any,
89+
'fetchRequestWithRetry',
90+
);
91+
92+
const response = await fetchClient.get('/vault/v1/kv', {});
93+
94+
expect(mockFetchRequestWithRetry).toHaveBeenCalledTimes(1);
95+
expect(fetchURL()).toBe('https://test.workos.com/vault/v1/kv');
96+
expect(await response.toJSON()).toEqual({ data: 'response' });
97+
});
98+
99+
it('post for Vault path should call fetchRequestWithRetry and return response', async () => {
100+
fetchOnce({ data: 'response' });
101+
const mockFetchRequestWithRetry = jest.spyOn(
102+
FetchHttpClient.prototype as any,
103+
'fetchRequestWithRetry',
104+
);
105+
106+
const response = await fetchClient.post('/vault/v1/kv', {}, {});
107+
108+
expect(mockFetchRequestWithRetry).toHaveBeenCalledTimes(1);
109+
expect(fetchURL()).toBe('https://test.workos.com/vault/v1/kv');
110+
expect(await response.toJSON()).toEqual({ data: 'response' });
111+
});
112+
113+
it('put for Vault path should call fetchRequestWithRetry and return response', async () => {
114+
fetchOnce({ data: 'response' });
115+
const mockFetchRequestWithRetry = jest.spyOn(
116+
FetchHttpClient.prototype as any,
117+
'fetchRequestWithRetry',
118+
);
119+
120+
const response = await fetchClient.put('/vault/v1/kv/secret', {}, {});
121+
122+
expect(mockFetchRequestWithRetry).toHaveBeenCalledTimes(1);
123+
expect(fetchURL()).toBe('https://test.workos.com/vault/v1/kv/secret');
124+
expect(await response.toJSON()).toEqual({ data: 'response' });
125+
});
126+
127+
it('delete for Vault path should call fetchRequestWithRetry and return response', async () => {
128+
fetchOnce({ data: 'response' });
129+
const mockFetchRequestWithRetry = jest.spyOn(
130+
FetchHttpClient.prototype as any,
131+
'fetchRequestWithRetry',
132+
);
133+
134+
const response = await fetchClient.delete('/vault/v1/kv/secret', {});
135+
136+
expect(mockFetchRequestWithRetry).toHaveBeenCalledTimes(1);
137+
expect(fetchURL()).toBe('https://test.workos.com/vault/v1/kv/secret');
138+
expect(await response.toJSON()).toEqual({ data: 'response' });
139+
});
140+
85141
it('should retry request on 500 status code', async () => {
86142
fetchOnce(
87143
{},

‎src/common/net/fetch-client.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface {
5151
options.params,
5252
);
5353

54-
if (path.startsWith('/fga/')) {
54+
if (HttpClient.isPathRetryable(path)) {
5555
return await this.fetchRequestWithRetry(
5656
resourceURL,
5757
'GET',
@@ -74,7 +74,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface {
7474
options.params,
7575
);
7676

77-
if (path.startsWith('/fga/')) {
77+
if (HttpClient.isPathRetryable(path)) {
7878
return await this.fetchRequestWithRetry(
7979
resourceURL,
8080
'POST',
@@ -108,7 +108,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface {
108108
options.params,
109109
);
110110

111-
if (path.startsWith('/fga/')) {
111+
if (HttpClient.isPathRetryable(path)) {
112112
return await this.fetchRequestWithRetry(
113113
resourceURL,
114114
'PUT',
@@ -141,7 +141,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface {
141141
options.params,
142142
);
143143

144-
if (path.startsWith('/fga/')) {
144+
if (HttpClient.isPathRetryable(path)) {
145145
return await this.fetchRequestWithRetry(
146146
resourceURL,
147147
'DELETE',

‎src/common/net/http-client.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export abstract class HttpClient implements HttpClientInterface {
1010
readonly MAX_RETRY_ATTEMPTS = 3;
1111
readonly BACKOFF_MULTIPLIER = 1.5;
1212
readonly MINIMUM_SLEEP_TIME_IN_MILLISECONDS = 500;
13-
readonly RETRY_STATUS_CODES = [500, 502, 504];
13+
readonly RETRY_STATUS_CODES = [408, 500, 502, 504];
1414

1515
constructor(readonly baseURL: string, readonly options?: RequestInit) {}
1616

@@ -88,6 +88,10 @@ export abstract class HttpClient implements HttpClientInterface {
8888
return JSON.stringify(entity);
8989
}
9090

91+
static isPathRetryable(path: string): boolean {
92+
return path.startsWith('/fga/') || path.startsWith('/vault/');
93+
}
94+
9195
private getSleepTimeInMilliseconds(retryAttempt: number): number {
9296
const sleepTime =
9397
this.MINIMUM_SLEEP_TIME_IN_MILLISECONDS *

‎src/common/net/node-client.spec.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,66 @@ describe('Node client', () => {
7878
expect(await response.toJSON()).toEqual({ data: 'response' });
7979
});
8080

81+
it('get for Vault path should call nodeRequestWithRetry and return response', async () => {
82+
nock('https://test.workos.com')
83+
.get('/vault/v1/kv')
84+
.reply(200, { data: 'response' });
85+
const mockNodeRequestWithRetry = jest.spyOn(
86+
NodeHttpClient.prototype as any,
87+
'nodeRequestWithRetry',
88+
);
89+
90+
const response = await nodeClient.get('/vault/v1/kv', {});
91+
92+
expect(mockNodeRequestWithRetry).toHaveBeenCalledTimes(1);
93+
expect(await response.toJSON()).toEqual({ data: 'response' });
94+
});
95+
96+
it('post for Vault path should call nodeRequestWithRetry and return response', async () => {
97+
nock('https://test.workos.com')
98+
.post('/vault/v1/kv')
99+
.reply(200, { data: 'response' });
100+
const mockNodeRequestWithRetry = jest.spyOn(
101+
NodeHttpClient.prototype as any,
102+
'nodeRequestWithRetry',
103+
);
104+
105+
const response = await nodeClient.post('/vault/v1/kv', {}, {});
106+
107+
expect(mockNodeRequestWithRetry).toHaveBeenCalledTimes(1);
108+
expect(await response.toJSON()).toEqual({ data: 'response' });
109+
});
110+
111+
it('put for Vault path should call nodeRequestWithRetry and return response', async () => {
112+
nock('https://test.workos.com')
113+
.put('/vault/v1/kv/secret')
114+
.reply(200, { data: 'response' });
115+
const mockNodeRequestWithRetry = jest.spyOn(
116+
NodeHttpClient.prototype as any,
117+
'nodeRequestWithRetry',
118+
);
119+
120+
const response = await nodeClient.put('/vault/v1/kv/secret', {}, {});
121+
122+
expect(mockNodeRequestWithRetry).toHaveBeenCalledTimes(1);
123+
expect(await response.toJSON()).toEqual({ data: 'response' });
124+
});
125+
126+
it('delete for Vault path should call nodeRequestWithRetry and return response', async () => {
127+
nock('https://test.workos.com')
128+
.delete('/vault/v1/kv/secret')
129+
.reply(200, { data: 'response' });
130+
const mockNodeRequestWithRetry = jest.spyOn(
131+
NodeHttpClient.prototype as any,
132+
'nodeRequestWithRetry',
133+
);
134+
135+
const response = await nodeClient.delete('/vault/v1/kv/secret', {});
136+
137+
expect(mockNodeRequestWithRetry).toHaveBeenCalledTimes(1);
138+
expect(await response.toJSON()).toEqual({ data: 'response' });
139+
});
140+
81141
it('should retry request on 500 status code', async () => {
82142
nock('https://test.workos.com')
83143
.get('/fga/v1/resources')

‎src/common/net/node-client.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export class NodeHttpClient extends HttpClient implements HttpClientInterface {
6262
options.params,
6363
);
6464

65-
if (path.startsWith('/fga/')) {
65+
if (HttpClient.isPathRetryable(path)) {
6666
return await this.nodeRequestWithRetry(
6767
resourceURL,
6868
'GET',
@@ -85,7 +85,7 @@ export class NodeHttpClient extends HttpClient implements HttpClientInterface {
8585
options.params,
8686
);
8787

88-
if (path.startsWith('/fga/')) {
88+
if (HttpClient.isPathRetryable(path)) {
8989
return await this.nodeRequestWithRetry(
9090
resourceURL,
9191
'POST',
@@ -119,7 +119,7 @@ export class NodeHttpClient extends HttpClient implements HttpClientInterface {
119119
options.params,
120120
);
121121

122-
if (path.startsWith('/fga/')) {
122+
if (HttpClient.isPathRetryable(path)) {
123123
return await this.nodeRequestWithRetry(
124124
resourceURL,
125125
'PUT',
@@ -152,7 +152,7 @@ export class NodeHttpClient extends HttpClient implements HttpClientInterface {
152152
options.params,
153153
);
154154

155-
if (path.startsWith('/fga/')) {
155+
if (HttpClient.isPathRetryable(path)) {
156156
return await this.nodeRequestWithRetry(
157157
resourceURL,
158158
'DELETE',

0 commit comments

Comments
 (0)