From 6b112d3fe86dafb6d6f86ac83357f67d8d75abf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 4 May 2026 10:43:49 +0200 Subject: [PATCH] fix(integration): correct GitLab fetch mode and retry on network errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes a misplaced `mode: 'same-origin'` option that would have rejected cross-origin requests when the integration is used from a browser, and extends the retry wrapper so transient network errors are retried using the configured `maxRetries`. Caller-initiated aborts still propagate immediately. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Fredrik Adelöw --- .changeset/gitlab-integration-fetch-fixes.md | 8 +++ .../src/gitlab/GitLabIntegration.test.ts | 63 +++++++++++++++++++ .../src/gitlab/GitLabIntegration.ts | 36 ++++++----- 3 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 .changeset/gitlab-integration-fetch-fixes.md diff --git a/.changeset/gitlab-integration-fetch-fixes.md b/.changeset/gitlab-integration-fetch-fixes.md new file mode 100644 index 0000000000..a00fa4d6fa --- /dev/null +++ b/.changeset/gitlab-integration-fetch-fixes.md @@ -0,0 +1,8 @@ +--- +'@backstage/integration': patch +--- + +Fixed two issues in the GitLab integration's fetch behavior: + +- The internal fetch wrapper was passing `mode: 'same-origin'` on every request. This had no practical effect server-side, but would have caused cross-origin requests to be rejected when the integration is used from a browser. Requests now use the default fetch mode and work correctly in both browser and Node environments. +- When retries are configured, transient network errors (such as dropped connections or DNS hiccups) are now retried using the same `maxRetries` and exponential delay as retryable HTTP status codes. Previously, a thrown fetch error would propagate immediately on the first failure regardless of the retry configuration. Caller-initiated aborts continue to surface immediately without being retried. diff --git a/packages/integration/src/gitlab/GitLabIntegration.test.ts b/packages/integration/src/gitlab/GitLabIntegration.test.ts index 7132849a7c..0d817a60c1 100644 --- a/packages/integration/src/gitlab/GitLabIntegration.test.ts +++ b/packages/integration/src/gitlab/GitLabIntegration.test.ts @@ -318,6 +318,69 @@ describe('GitLabIntegration', () => { expect(response.status).toBe(200); expect(callCount).toBe(3); }); + + it('retries on transient network errors and returns once recovered', async () => { + let callCount = 0; + worker.use( + rest.get('https://h.com/api/v4', (_req, res, ctx) => { + callCount += 1; + if (callCount === 1) { + return res.networkError('boom'); + } + return res(ctx.status(200), ctx.json({})); + }), + ); + + const integration = new GitLabIntegration({ + host: 'h.com', + apiBaseUrl: 'https://h.com/api/v4', + baseUrl: 'https://h.com', + retry: { + maxRetries: 3, + retryStatusCodes: [429], + }, + }); + + const responsePromise = integration.fetch('https://h.com/api/v4'); + await jest.advanceTimersByTimeAsync(100); + const response = await responsePromise; + + expect(response.status).toBe(200); + expect(callCount).toBe(2); + }); + + it('surfaces the error after exhausting retries on persistent network errors', async () => { + let callCount = 0; + worker.use( + rest.get('https://h.com/api/v4', (_req, res) => { + callCount += 1; + return res.networkError('boom'); + }), + ); + + const integration = new GitLabIntegration({ + host: 'h.com', + apiBaseUrl: 'https://h.com/api/v4', + baseUrl: 'https://h.com', + retry: { + maxRetries: 2, + retryStatusCodes: [429], + }, + }); + + // Attach the catch handler before advancing timers so the in-flight + // rejections don't surface as unhandled. + const caught = integration + .fetch('https://h.com/api/v4') + .catch((e: unknown) => e); + await jest.advanceTimersByTimeAsync(100); + await jest.advanceTimersByTimeAsync(200); + const error = await caught; + + expect(error).toBeTruthy(); + expect(String(error)).toMatch(/fetch/i); + expect(callCount).toBe(3); // initial + 2 retries + }); }); }); diff --git a/packages/integration/src/gitlab/GitLabIntegration.ts b/packages/integration/src/gitlab/GitLabIntegration.ts index 19c349af9e..9b326f3dea 100644 --- a/packages/integration/src/gitlab/GitLabIntegration.ts +++ b/packages/integration/src/gitlab/GitLabIntegration.ts @@ -75,9 +75,7 @@ export class GitLabIntegration implements ScmIntegration { } private createFetchStrategy(): FetchFunction { - let fetchFn: FetchFunction = async (url, options) => { - return fetch(url, { ...options, mode: 'same-origin' }); - }; + let fetchFn: FetchFunction = fetch; const retryConfig = this.integrationConfig.retry; if (retryConfig) { @@ -111,29 +109,39 @@ export class GitLabIntegration implements ScmIntegration { return async (url, options) => { const abortSignal = options?.signal; - let response: Response; let attempt = 0; for (;;) { - response = await fetchFn(url, options); - // If response is not retryable, return immediately - if (!retryStatusCodes.includes(response.status)) { - break; + let response: Response | undefined; + let error: unknown; + try { + response = await fetchFn(url, options); + } catch (e) { + // The caller aborted — surface that immediately rather than retrying. + if (abortSignal?.aborted) { + throw e; + } + error = e; } - // If this was the last allowed attempt, return response - if (attempt++ >= maxRetries) { - break; + // Successful, non-retryable response: return immediately + if (response && !retryStatusCodes.includes(response.status)) { + return response; } + + // Out of attempts: surface the response or rethrow the captured error + if (attempt++ >= maxRetries) { + if (error) throw error; + return response!; + } + // Determine delay from Retry-After header if present, otherwise exponential backoff - const retryAfter = response.headers.get('Retry-After'); + const retryAfter = response?.headers.get('Retry-After'); const delay = retryAfter ? parseInt(retryAfter, 10) * 1000 : Math.min(100 * Math.pow(2, attempt - 1), 10000); // Exponential backoff, cap at 10 seconds await sleep(delay, abortSignal); } - - return response; }; } }