Skip to content

Commit 5c0e70a

Browse files
author
Roman Snapko
authored
Add retry logic for MS Outlook trigger (#2093)
<!-- Ensure the title clearly reflects what was changed. Provide a clear and concise description of the changes made. The PR should only contain the changes related to the issue, and no other unrelated changes. --> Fixes OPS-3295 The error comes from ms teams and cannot be reproduced with the same data (we don't have an error on the next run). So my guess is to add retry on the trigger and keep tracking it
1 parent 5a28052 commit 5c0e70a

3 files changed

Lines changed: 312 additions & 2 deletions

File tree

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import { GraphError } from '@microsoft/microsoft-graph-client';
2+
import { logger } from '@openops/server-shared';
3+
4+
export type RetryOptions = {
5+
maxRetries: number;
6+
initialDelayMs: number;
7+
shouldRetry: (error: unknown) => boolean;
8+
onRetry?: (error: unknown, attempt: number, delayMs: number) => void;
9+
};
10+
11+
export async function withGraphRetry<T>(
12+
fn: () => Promise<T>,
13+
options: RetryOptions,
14+
): Promise<T> {
15+
const { maxRetries, initialDelayMs, shouldRetry, onRetry } = options;
16+
17+
if (maxRetries < 1) {
18+
throw new Error(`maxRetries must be at least 1, but got ${maxRetries}`);
19+
}
20+
21+
if (initialDelayMs < 0) {
22+
throw new Error(
23+
`initialDelayMs must be non-negative, but got ${initialDelayMs}`,
24+
);
25+
}
26+
27+
let lastError: unknown;
28+
29+
for (let i = 0; i < maxRetries; i++) {
30+
try {
31+
return await fn();
32+
} catch (e: unknown) {
33+
lastError = e;
34+
35+
if (!shouldRetry(e) || i === maxRetries - 1) {
36+
throw e;
37+
}
38+
39+
const delay = initialDelayMs * Math.pow(2, i);
40+
if (onRetry) {
41+
onRetry(e, i + 1, delay);
42+
}
43+
44+
await new Promise((resolve) => setTimeout(resolve, delay));
45+
}
46+
}
47+
48+
throw lastError;
49+
}
50+
51+
export const DEFAULT_MAX_RETRIES = 3;
52+
export const DEFAULT_INITIAL_DELAY_MS = 1000;
53+
54+
export type MicrosoftGraphRetryOptions = {
55+
maxRetries?: number;
56+
initialDelayMs?: number;
57+
};
58+
59+
export async function microsoftGraphRetry<T>(
60+
fn: () => Promise<T>,
61+
options: MicrosoftGraphRetryOptions = {},
62+
): Promise<T> {
63+
const {
64+
maxRetries = DEFAULT_MAX_RETRIES,
65+
initialDelayMs = DEFAULT_INITIAL_DELAY_MS,
66+
} = options;
67+
68+
return withGraphRetry(fn, {
69+
maxRetries,
70+
initialDelayMs,
71+
shouldRetry: (e) => {
72+
const isGraphError = e instanceof GraphError;
73+
const statusCode = isGraphError ? e.statusCode : undefined;
74+
return statusCode !== undefined && statusCode >= 500;
75+
},
76+
onRetry: (e, attempt, delay) => {
77+
const isGraphError = e instanceof GraphError;
78+
const statusCode = isGraphError ? e.statusCode : undefined;
79+
logger.warn(
80+
`Transient Microsoft Graph error ${statusCode} occurred. Retrying in ${delay}ms... (Attempt ${attempt}/${maxRetries})`,
81+
);
82+
},
83+
});
84+
}

packages/blocks/microsoft-outlook/src/lib/triggers/new-email.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
import { isEmpty, isString } from '@openops/shared';
1111
import dayjs from 'dayjs';
1212
import { microsoftOutlookAuth } from '../common/auth';
13+
import { microsoftGraphRetry } from '../common/graph-retry';
1314
import { mailFolderIdDropdown } from '../common/props';
1415

1516
function normalizeString(value: string): string {
@@ -146,15 +147,17 @@ async function fetchMessages(
146147
);
147148
}
148149

149-
let response: PageCollection = await req.get();
150+
let response: PageCollection = await microsoftGraphRetry(() => req.get());
150151
do {
151152
messages.push(...(response.value as Message[]));
152153

153154
if (!shouldFetchAll || !response['@odata.nextLink']) {
154155
break;
155156
}
156157

157-
response = await client.api(response['@odata.nextLink']).get();
158+
response = await microsoftGraphRetry(() =>
159+
client.api(response['@odata.nextLink'] as string).get(),
160+
);
158161
} while (response.value.length > 0);
159162

160163
return messages;
Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
import { GraphError } from '@microsoft/microsoft-graph-client';
2+
import {
3+
microsoftGraphRetry,
4+
withGraphRetry,
5+
} from '../src/lib/common/graph-retry';
6+
7+
describe('withGraphRetry', () => {
8+
it('should return the result if fn succeeds on the first attempt', async () => {
9+
const fn = jest.fn().mockResolvedValue('success');
10+
const result = await withGraphRetry(fn, {
11+
maxRetries: 3,
12+
initialDelayMs: 1,
13+
shouldRetry: () => true,
14+
});
15+
expect(result).toBe('success');
16+
expect(fn).toHaveBeenCalledTimes(1);
17+
});
18+
19+
it('should retry and succeed if fn fails initially', async () => {
20+
const fn = jest
21+
.fn()
22+
.mockRejectedValueOnce(new Error('fail'))
23+
.mockResolvedValueOnce('success');
24+
const onRetry = jest.fn();
25+
26+
const result = await withGraphRetry(fn, {
27+
maxRetries: 3,
28+
initialDelayMs: 1,
29+
shouldRetry: () => true,
30+
onRetry,
31+
});
32+
33+
expect(result).toBe('success');
34+
expect(fn).toHaveBeenCalledTimes(2);
35+
expect(onRetry).toHaveBeenCalledTimes(1);
36+
expect(onRetry).toHaveBeenNthCalledWith(1, expect.any(Error), 1, 1);
37+
});
38+
39+
it('should throw the error if fn fails and shouldRetry returns false', async () => {
40+
const error = new Error('critical fail');
41+
const fn = jest.fn().mockRejectedValue(error);
42+
const shouldRetry = jest.fn().mockReturnValue(false);
43+
44+
await expect(
45+
withGraphRetry(fn, {
46+
maxRetries: 3,
47+
initialDelayMs: 1,
48+
shouldRetry,
49+
}),
50+
).rejects.toThrow(error);
51+
52+
expect(fn).toHaveBeenCalledTimes(1);
53+
expect(shouldRetry).toHaveBeenCalledWith(error);
54+
});
55+
56+
it('should throw the last error after reaching max retries', async () => {
57+
const error = new Error('persistent fail');
58+
const fn = jest.fn().mockRejectedValue(error);
59+
const onRetry = jest.fn();
60+
61+
await expect(
62+
withGraphRetry(fn, {
63+
maxRetries: 3,
64+
initialDelayMs: 1,
65+
shouldRetry: () => true,
66+
onRetry,
67+
}),
68+
).rejects.toThrow(error);
69+
70+
expect(fn).toHaveBeenCalledTimes(3);
71+
expect(onRetry).toHaveBeenCalledTimes(2);
72+
});
73+
74+
it('should use exponential backoff for delays', async () => {
75+
jest.useFakeTimers();
76+
const fn = jest
77+
.fn()
78+
.mockRejectedValueOnce(new Error('fail 1'))
79+
.mockRejectedValueOnce(new Error('fail 2'))
80+
.mockResolvedValueOnce('success');
81+
const onRetry = jest.fn();
82+
83+
const promise = withGraphRetry(fn, {
84+
maxRetries: 3,
85+
initialDelayMs: 10,
86+
shouldRetry: () => true,
87+
onRetry,
88+
});
89+
90+
await Promise.resolve();
91+
await Promise.resolve();
92+
await Promise.resolve();
93+
expect(fn).toHaveBeenCalledTimes(1);
94+
95+
jest.advanceTimersByTime(10);
96+
await Promise.resolve();
97+
await Promise.resolve();
98+
await Promise.resolve();
99+
expect(fn).toHaveBeenCalledTimes(2);
100+
expect(onRetry).toHaveBeenNthCalledWith(1, expect.any(Error), 1, 10);
101+
102+
jest.advanceTimersByTime(20);
103+
await Promise.resolve();
104+
await Promise.resolve();
105+
expect(fn).toHaveBeenCalledTimes(3);
106+
expect(onRetry).toHaveBeenNthCalledWith(2, expect.any(Error), 2, 20);
107+
108+
const result = await promise;
109+
expect(result).toBe('success');
110+
111+
jest.useRealTimers();
112+
});
113+
114+
it('should throw error if maxRetries is less than 1', async () => {
115+
const fn = jest.fn();
116+
await expect(
117+
withGraphRetry(fn, {
118+
maxRetries: 0,
119+
initialDelayMs: 1,
120+
shouldRetry: () => true,
121+
}),
122+
).rejects.toThrow('maxRetries must be at least 1, but got 0');
123+
});
124+
125+
it('should throw error if initialDelayMs is negative', async () => {
126+
const fn = jest.fn();
127+
await expect(
128+
withGraphRetry(fn, {
129+
maxRetries: 1,
130+
initialDelayMs: -1,
131+
shouldRetry: () => true,
132+
}),
133+
).rejects.toThrow('initialDelayMs must be non-negative, but got -1');
134+
});
135+
});
136+
137+
describe('microsoftGraphRetry', () => {
138+
it('should retry on 500+ errors', async () => {
139+
const error500 = new GraphError();
140+
error500.statusCode = 500;
141+
142+
const error502 = new GraphError();
143+
error502.statusCode = 502;
144+
145+
const fn = jest
146+
.fn()
147+
.mockRejectedValueOnce(error500)
148+
.mockRejectedValueOnce(error502)
149+
.mockResolvedValueOnce('success');
150+
151+
const result = await microsoftGraphRetry(fn, { initialDelayMs: 0 });
152+
153+
expect(result).toBe('success');
154+
expect(fn).toHaveBeenCalledTimes(3);
155+
});
156+
157+
it('should use default values if options are not provided', async () => {
158+
const error502 = new GraphError();
159+
error502.statusCode = 502;
160+
161+
const fn = jest
162+
.fn()
163+
.mockRejectedValueOnce(error502)
164+
.mockRejectedValueOnce(error502)
165+
.mockResolvedValueOnce('success');
166+
167+
jest.useFakeTimers();
168+
const promise = microsoftGraphRetry(fn);
169+
170+
await Promise.resolve();
171+
await Promise.resolve();
172+
await Promise.resolve();
173+
jest.advanceTimersByTime(1000);
174+
await Promise.resolve();
175+
await Promise.resolve();
176+
await Promise.resolve();
177+
jest.advanceTimersByTime(2000);
178+
await Promise.resolve();
179+
await Promise.resolve();
180+
181+
const result = await promise;
182+
expect(result).toBe('success');
183+
expect(fn).toHaveBeenCalledTimes(3);
184+
jest.useRealTimers();
185+
});
186+
187+
it('should allow overriding default values', async () => {
188+
const error502 = new GraphError();
189+
error502.statusCode = 502;
190+
191+
const fn = jest.fn().mockRejectedValue(error502);
192+
193+
await expect(
194+
microsoftGraphRetry(fn, {
195+
maxRetries: 2,
196+
initialDelayMs: 0,
197+
}),
198+
).rejects.toThrow(error502);
199+
200+
expect(fn).toHaveBeenCalledTimes(2);
201+
});
202+
203+
it('should not retry on other errors (e.g., 401)', async () => {
204+
const error401 = new GraphError();
205+
error401.statusCode = 401;
206+
207+
const fn = jest.fn().mockRejectedValue(error401);
208+
209+
await expect(microsoftGraphRetry(fn)).rejects.toThrow(error401);
210+
211+
expect(fn).toHaveBeenCalledTimes(1);
212+
});
213+
214+
it('should not retry on non-GraphError', async () => {
215+
const error = new Error('not a graph error');
216+
217+
const fn = jest.fn().mockRejectedValue(error);
218+
219+
await expect(microsoftGraphRetry(fn)).rejects.toThrow(error);
220+
221+
expect(fn).toHaveBeenCalledTimes(1);
222+
});
223+
});

0 commit comments

Comments
 (0)