Skip to content

Commit

Permalink
Do not retry POST/PUT requests by default
Browse files Browse the repository at this point in the history
  • Loading branch information
tpmcgowan committed Jun 15, 2023
1 parent 207f484 commit c8daf03
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 19 deletions.
118 changes: 118 additions & 0 deletions server/data/restClient.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import nock from 'nock'
import RestClient from './restClient'
import { AgentConfig } from '../config'

const restClient = new RestClient(
'name-1',
{
url: 'http://localhost:8080/api',
timeout: {
response: 1000,
deadline: 1000,
},
agent: new AgentConfig(1000),
},
'token-1',
)

describe('POST', () => {
it('Should return response body', async () => {
nock('http://localhost:8080', {
reqheaders: { authorization: 'Bearer token-1' },
})
.post('/api/test')
.reply(200, { success: true })

const result = await restClient.post({
path: '/test',
})

expect(nock.isDone()).toBe(true)

expect(result).toStrictEqual({
success: true,
})
})

it('Should return raw response body', async () => {
nock('http://localhost:8080', {
reqheaders: { authorization: 'Bearer token-1' },
})
.post('/api/test')
.reply(200, { success: true })

const result = await restClient.post({
path: '/test',
headers: { header1: 'headerValue1' },
raw: true,
})

expect(nock.isDone()).toBe(true)

expect(result).toMatchObject({
req: { method: 'POST' },
status: 200,
text: '{"success":true}',
})
})

it('Should not retry by default', async () => {
nock('http://localhost:8080', {
reqheaders: { authorization: 'Bearer token-1' },
})
.post('/api/test')
.reply(500)

await expect(
restClient.post({
path: '/test',
headers: { header1: 'headerValue1' },
}),
).rejects.toThrow('Internal Server Error')

expect(nock.isDone()).toBe(true)
})

it('retries if configured to do so', async () => {
nock('http://localhost:8080', {
reqheaders: { authorization: 'Bearer token-1' },
})
.post('/api/test')
.reply(500)
.post('/api/test')
.reply(500)
.post('/api/test')
.reply(500)

await expect(
restClient.post({
path: '/test',
headers: { header1: 'headerValue1' },
retry: true,
}),
).rejects.toThrow('Internal Server Error')

expect(nock.isDone()).toBe(true)
})

it('can recover through retries', async () => {
nock('http://localhost:8080', {
reqheaders: { authorization: 'Bearer token-1' },
})
.post('/api/test')
.reply(500)
.post('/api/test')
.reply(500)
.post('/api/test')
.reply(200, { success: true })

const result = await restClient.post({
path: '/test',
headers: { header1: 'headerValue1' },
retry: true,
})

expect(result).toStrictEqual({ success: true })
expect(nock.isDone()).toBe(true)
})
})
18 changes: 17 additions & 1 deletion server/data/restClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ interface PostRequest {
responseType?: string
data?: Record<string, unknown>
raw?: boolean
retry?: boolean
}

interface PutRequest {
Expand All @@ -29,6 +30,7 @@ interface PutRequest {
responseType?: string
data?: Record<string, unknown>
raw?: boolean
retry?: boolean
}

interface StreamRequest {
Expand Down Expand Up @@ -82,6 +84,7 @@ export default class RestClient {
responseType = '',
data = {},
raw = false,
retry = false,
}: PostRequest = {}): Promise<T> {
logger.info(`Post using user credentials: calling ${this.name}: ${path}`)
try {
Expand All @@ -90,6 +93,9 @@ export default class RestClient {
.send(data)
.agent(this.agent)
.retry(2, (err, res) => {
if (retry === false) {
return false
}
if (err) logger.info(`Retry handler found API error with ${err.code} ${err.message}`)
return undefined // retry handler only for logging retries, not to influence retry logic
})
Expand All @@ -106,14 +112,24 @@ export default class RestClient {
}
}

async put<T>({ path = null, headers = {}, responseType = '', data = {}, raw = false }: PutRequest = {}): Promise<T> {
async put<T>({
path = null,
headers = {},
responseType = '',
data = {},
raw = false,
retry = false,
}: PutRequest = {}): Promise<T> {
logger.info(`Put using user credentials: calling ${this.name}: ${path}`)
try {
const result = await superagent
.put(`${this.apiUrl()}${path}`)
.send(data)
.agent(this.agent)
.retry(2, (err, res) => {
if (retry === false) {
return false
}
if (err) logger.info(`Retry handler found API error with ${err.code} ${err.message}`)
return undefined // retry handler only for logging retries, not to influence retry logic
})
Expand Down
19 changes: 10 additions & 9 deletions server/sanitisedError.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import sanitisedError, { UnsanitisedError } from './sanitisedError'
import sanitisedError, { UnsanitisedError, SanitisedError } from './sanitisedError'

describe('sanitised error', () => {
it('it should omit the request headers from the error object ', () => {
Expand All @@ -25,14 +25,15 @@ describe('sanitised error', () => {
stack: 'stack description',
} as unknown as UnsanitisedError

expect(sanitisedError(error)).toEqual({
headers: { date: 'Tue, 19 May 2020 15:16:20 GMT' },
message: 'Not Found',
stack: 'stack description',
status: 404,
text: { details: 'details' },
data: { content: 'hello' },
})
const e = new Error() as SanitisedError
e.message = 'Not Found'
e.text = 'details'
e.status = 404
e.headers = { date: 'Tue, 19 May 2020 15:16:20 GMT' }
e.data = { content: 'hello' }
e.stack = 'stack description'

expect(sanitisedError(error)).toEqual(e)
})

it('it should return the error message ', () => {
Expand Down
18 changes: 9 additions & 9 deletions server/sanitisedError.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { ResponseError } from 'superagent'

interface SanitisedError {
export interface SanitisedError {
text?: string
status?: number
headers?: unknown
Expand All @@ -13,14 +13,14 @@ export type UnsanitisedError = ResponseError

export default function sanitise(error: UnsanitisedError): SanitisedError {
if (error.response) {
return {
text: error.response.text,
status: error.response.status,
headers: error.response.headers,
data: error.response.body,
message: error.message,
stack: error.stack,
}
const e = new Error(error.message) as SanitisedError
e.text = error.response.text
e.status = error.response.status
e.headers = error.response.headers
e.data = error.response.body
e.message = error.message
e.stack = error.stack
return e
}
return {
message: error.message,
Expand Down

0 comments on commit c8daf03

Please sign in to comment.