Skip to content

Commit 9f655fa

Browse files
authored
chore: add automatic retries for 502s from M365 (#863)
## Problem User reported receiving 502 bad gateway error for M365 step and had to manually retry for the step / execution to work. ## Solution **Improvements**: - Add automatic retries for 502s (already retrying for 500s and 503s)
1 parent 8a4019d commit 9f655fa

File tree

2 files changed

+23
-19
lines changed

2 files changed

+23
-19
lines changed

packages/backend/src/apps/m365-excel/__tests__/common/interceptors.error-handlers.test.ts

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -97,23 +97,26 @@ describe('M365 request error handlers', () => {
9797
vi.restoreAllMocks()
9898
})
9999

100-
it('logs a warning and throws a RetriableError with default step delay on 503, if response does not have retry-after', async () => {
101-
mockAxiosAdapterToThrowOnce(503)
102-
await http
103-
.get('/test-url')
104-
.then(() => {
105-
expect.unreachable()
106-
})
107-
.catch((error): void => {
108-
expect(error).toBeInstanceOf(RetriableError)
109-
expect(error.delayType).toEqual('step')
110-
expect(error.delayInMs).toEqual(DEFAULT_DELAY_MS)
111-
})
112-
expect(mocks.logWarning).toHaveBeenCalledWith(
113-
expect.stringContaining('HTTP 503'),
114-
expect.objectContaining({ event: 'm365-http-503' }),
115-
)
116-
})
100+
it.each([500, 502, 503])(
101+
'logs a warning and throws a RetriableError with default step delay on %s',
102+
async (status) => {
103+
mockAxiosAdapterToThrowOnce(status)
104+
await http
105+
.get('/test-url')
106+
.then(() => {
107+
expect.unreachable()
108+
})
109+
.catch((error): void => {
110+
expect(error).toBeInstanceOf(RetriableError)
111+
expect(error.delayType).toEqual('step')
112+
expect(error.delayInMs).toEqual(DEFAULT_DELAY_MS)
113+
})
114+
expect(mocks.logWarning).toHaveBeenCalledWith(
115+
expect.stringContaining(`HTTP ${status}`),
116+
expect.objectContaining({ event: `m365-http-${status}` }),
117+
)
118+
},
119+
)
117120

118121
it('logs a warning and throws a RetriableError with step delay set to retry-after, on receiving 503', async () => {
119122
mockAxiosAdapterToThrowOnce(503, { 'retry-after': 123 })

packages/backend/src/apps/m365-excel/common/interceptors/request-error-handler.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ const handle429: ThrowingHandler = ($, error) => {
5959
//
6060
// Retry failures due to flakey M365 servers
6161
//
62-
const handle500and503: ThrowingHandler = function ($, error) {
62+
const handle500and502and503: ThrowingHandler = function ($, error) {
6363
// Log to monitor spikes, just in case
6464
const status = error.response.status
6565
logger.warn(`Received HTTP ${status} from MS Graph`, {
@@ -115,8 +115,9 @@ const errorHandler: IApp['requestErrorHandler'] = async function ($, error) {
115115
case 429: // Rate limited
116116
return handle429($, error)
117117
case 500:
118+
case 502: // Bad gateway
118119
case 503: // Transient error
119-
return handle500and503($, error)
120+
return handle500and502and503($, error)
120121
case 509: // Bandwidth limit reached
121122
return handle509($, error)
122123
}

0 commit comments

Comments
 (0)