diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/methods/listSearch.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/methods/listSearch.ts index 02ded4fabab..8cbde108be6 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/methods/listSearch.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/methods/listSearch.ts @@ -40,22 +40,30 @@ export async function getChats( $expand: 'members', }; + // `/v1.0/chats` occasionally 5xxs transiently; retry up to `maxAttempts` times, + // sleeping 1s between attempts (not after the last one), and surface the final + // failure instead of swallowing it as an empty result. + const maxAttempts = 5; let value: IDataObject[] = []; - let attempts = 5; - do { + let lastError: Error | undefined; + + for (let attempt = 0; attempt < maxAttempts; attempt++) { try { value = ((await microsoftApiRequest.call(this, 'GET', '/v1.0/chats', {}, qs)) as IDataObject) .value as IDataObject[]; + lastError = undefined; break; } catch (error) { - if (attempts > 0) { + lastError = error; + if (attempt < maxAttempts - 1) { await sleep(1000); - attempts--; - } else { - throw new NodeOperationError(this.getNode(), error); } } - } while (attempts > 0); + } + + if (lastError) { + throw new NodeOperationError(this.getNode(), lastError); + } for (const chat of value) { if (!chat.topic) { diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/test/methods/listSearch.test.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/test/methods/listSearch.test.ts new file mode 100644 index 00000000000..9c648b85f11 --- /dev/null +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/test/methods/listSearch.test.ts @@ -0,0 +1,116 @@ +import type { ILoadOptionsFunctions, INode } from 'n8n-workflow'; +import { sleep } from '@n8n/utils/sleep'; +import type { Mock } from 'vitest'; +import type { DeepMockProxy } from 'vitest-mock-extended'; +import { mock, mockDeep } from 'vitest-mock-extended'; + +import { getChats } from '../../methods/listSearch'; +import * as transport from '../../transport'; +import type * as _importType0 from '../../transport'; + +// Real transport module except the network helper +vi.mock('../../transport', async () => { + const originalModule = await vi.importActual('../../transport'); + return { + ...originalModule, + microsoftApiRequest: vi.fn(), + }; +}); + +// `getChats` retries through `sleep(1000)` between attempts; stub it so the +// retry-count/no-final-sleep assertions below run instantly instead of for real seconds. +vi.mock('@n8n/utils/sleep', () => ({ + sleep: vi.fn().mockResolvedValue(undefined), +})); + +// Service Principal guard coverage for `getChats` already lives in +// `../transport/index.test.ts` (`'listSearch credential routing'`), via a different +// mocking layer (low-level HTTP helpers). Not duplicated here. +describe('Microsoft Teams v2 — getChats', () => { + let ctx: DeepMockProxy; + const apiRequest = transport.microsoftApiRequest as Mock; + const sleepMock = sleep as unknown as Mock; + + const setParams = (params: Record) => { + ctx.getNodeParameter.mockImplementation( + (name: string, _itemIndex?: number, fallback?: unknown) => + (name in params ? params[name] : fallback) as never, + ); + }; + + beforeEach(() => { + vi.clearAllMocks(); + ctx = mockDeep(); + ctx.getNode.mockReturnValue(mock({ typeVersion: 1 })); + setParams({ authentication: 'microsoftOAuth2Api' }); + }); + + it('throws a NodeOperationError carrying the original Graph error message when every attempt fails', async () => { + apiRequest.mockRejectedValue(new Error('Graph is down')); + + await expect(getChats.call(ctx)).rejects.toThrow('Graph is down'); + }); + + it('pins the retry count at exactly 5 calls and sleeps exactly 4 times (not 5) on total failure', async () => { + apiRequest.mockRejectedValue(new Error('boom')); + + await expect(getChats.call(ctx)).rejects.toThrow(); + expect(apiRequest).toHaveBeenCalledTimes(5); + expect(sleepMock).toHaveBeenCalledTimes(4); + }); + + it('recovers if the first 4 attempts fail, resolving with the parsed chats and no throw', async () => { + apiRequest + .mockRejectedValueOnce(new Error('transient 1')) + .mockRejectedValueOnce(new Error('transient 2')) + .mockRejectedValueOnce(new Error('transient 3')) + .mockRejectedValueOnce(new Error('transient 4')) + .mockResolvedValueOnce({ + value: [{ id: 'c1', topic: 'Standup', chatType: 'Group', webUrl: 'https://teams/chat/c1' }], + }); + + const result = await getChats.call(ctx); + + expect(apiRequest).toHaveBeenCalledTimes(5); + expect(sleepMock).toHaveBeenCalledTimes(4); + expect(result.results).toEqual([ + { name: 'Standup (Group)', value: 'c1', url: 'https://teams/chat/c1' }, + ]); + }); + + it('succeeds on the first attempt with exactly 1 call and no sleep', async () => { + apiRequest.mockResolvedValue({ value: [] }); + + await getChats.call(ctx); + + expect(apiRequest).toHaveBeenCalledTimes(1); + expect(sleepMock).not.toHaveBeenCalled(); + }); + + it('returns an empty result for a genuinely empty tenant without throwing', async () => { + apiRequest.mockResolvedValue({ value: [] }); + + const result = await getChats.call(ctx); + + expect(result).toEqual({ results: [] }); + }); + + it('builds the chat name from member display names when topic is absent, preserving the chatType suffix', async () => { + apiRequest.mockResolvedValue({ + value: [ + { + id: 'c2', + chatType: 'oneOnOne', + webUrl: 'https://teams/chat/c2', + members: [{ displayName: 'Alice' }, { displayName: 'Bob' }, {}], + }, + ], + }); + + const result = await getChats.call(ctx); + + expect(result.results).toEqual([ + { name: 'Alice, Bob (oneOnOne)', value: 'c2', url: 'https://teams/chat/c2' }, + ]); + }); +});