refactor(worker): normalize HTTP error metadata access

Author: brendan-kellam

packages/backend/src/ee/permissionSyncError.ts

@@ -1,5 +1,5 @@
 import type { IdentityProviderType } from '@sourcebot/shared';
-import { getErrorStatus } from '../errors.js';
+import { getErrorHeader, getErrorStatus } from '../errors.js';
 
 export type PermissionSyncUpstreamErrorKind =
     | 'credential_rejected'
@@ -38,31 +38,10 @@ export class PermissionSyncUpstreamError extends Error {
     }
 }
 
-const getHeader = (error: unknown, name: string): string | undefined => {
-    if (error === null || typeof error !== 'object') {
-        return undefined;
-    }
-
-    const directHeaders = (error as { response?: { headers?: unknown } }).response?.headers;
-    const nestedHeaders = (error as { cause?: { response?: { headers?: unknown } } }).cause?.response?.headers;
-    const headers = directHeaders ?? nestedHeaders;
-
-    if (headers instanceof Headers) {
-        return headers.get(name) ?? undefined;
-    }
-
-    if (headers !== null && typeof headers === 'object') {
-        const value = (headers as Record<string, unknown>)[name.toLowerCase()];
-        return typeof value === 'string' ? value : undefined;
-    }
-
-    return undefined;
-};
-
 const isRateLimited = (error: unknown, status: number | null): boolean =>
     status === 429 ||
-    getHeader(error, 'retry-after') !== undefined ||
-    getHeader(error, 'x-ratelimit-remaining') === '0';
+    getErrorHeader(error, 'retry-after') !== undefined ||
+    getErrorHeader(error, 'x-ratelimit-remaining') === '0';
 
 const isNetworkOrTimeoutError = (error: unknown): boolean => {
     if (!(error instanceof Error)) {

packages/backend/src/errors.test.ts

@@ -1,7 +1,7 @@
 import { describe, expect, test } from 'vitest';
 import { RequestError } from '@octokit/request-error';
 import { GitbeakerRequestError } from '@gitbeaker/requester-utils';
-import { isForbidden, isGone, isUnauthorized } from './errors';
+import { getErrorHeader, getErrorStatus, isForbidden, isGone, isUnauthorized } from './errors';
 import { throwOnHttpError } from './bitbucket';
 
 // Helper: invoke the openapi-fetch middleware against a synthetic Response and
@@ -23,6 +23,59 @@ const invokeMiddleware = async (response: Response): Promise<unknown> => {
     }
 };
 
+describe('HTTP error metadata', () => {
+    test('reads status and case-insensitive headers from a direct response', () => {
+        const error = Object.assign(new Error('Rate limited'), {
+            response: {
+                status: 429,
+                headers: {
+                    'Retry-After': '30',
+                },
+            },
+        });
+
+        expect(getErrorStatus(error)).toBe(429);
+        expect(getErrorHeader(error, 'retry-after')).toBe('30');
+    });
+
+    test('reads status and native Headers from a nested cause response', () => {
+        const error = Object.assign(new Error('Rate limited'), {
+            cause: {
+                response: new Response(null, {
+                    status: 429,
+                    headers: {
+                        'X-RateLimit-Remaining': '0',
+                    },
+                }),
+            },
+        });
+
+        expect(getErrorStatus(error)).toBe(429);
+        expect(getErrorHeader(error, 'x-ratelimit-remaining')).toBe('0');
+    });
+
+    test('combines a direct status with headers from the direct response', () => {
+        const error = Object.assign(new Error('Rate limited'), {
+            status: 403,
+            response: {
+                headers: {
+                    'x-ratelimit-remaining': '0',
+                },
+            },
+        });
+
+        expect(getErrorStatus(error)).toBe(403);
+        expect(getErrorHeader(error, 'X-RateLimit-Remaining')).toBe('0');
+    });
+
+    test('returns no metadata for a plain error', () => {
+        const error = new Error('Not an HTTP error');
+
+        expect(getErrorStatus(error)).toBeNull();
+        expect(getErrorHeader(error, 'retry-after')).toBeUndefined();
+    });
+});
+
 describe('isUnauthorized', () => {
     test('Octokit RequestError with status 401', () => {
         const err = new RequestError('Unauthorized', 401, {

packages/backend/src/errors.ts

@@ -1,28 +1,56 @@
 
+type HttpErrorDetails = {
+    status: number | null;
+    headers?: unknown;
+};
+
 /**
- * Extract an HTTP status code from a thrown error across the libraries used by
- * the code-host clients:
- *   - Octokit RequestError: { status }
- *   - openapi-fetch (Bitbucket Cloud / Server): direct throws with { status }
- *     or errors wrapped via Object.assign(new Error(...), { status })
- *   - gitbeaker (GitLab): { cause: { response: { status } } }
+ * Normalizes HTTP response metadata across the code-host client libraries:
+ *   - Octokit RequestError: { status, response: { headers } }
+ *   - openapi-fetch (Bitbucket Cloud / Server): { status }
+ *   - gitbeaker (GitLab): { cause: { response: { status, headers } } }
  */
-export const getErrorStatus = (err: unknown): number | null => {
-    if (err === null || typeof err !== 'object') {
-        return null;
+const getHttpErrorDetails = (error: unknown): HttpErrorDetails => {
+    if (error === null || typeof error !== 'object') {
+        return { status: null };
     }
 
-    const direct = (err as { status?: unknown }).status;
-    if (typeof direct === 'number') {
-        return direct;
+    const directError = error as {
+        status?: unknown;
+        response?: { status?: unknown; headers?: unknown };
+        cause?: { response?: { status?: unknown; headers?: unknown } };
+    };
+    const directResponse = directError.response;
+    const nestedResponse = directError.cause?.response;
+    const status = [
+        directError.status,
+        directResponse?.status,
+        nestedResponse?.status,
+    ].find((value): value is number => typeof value === 'number') ?? null;
+
+    return {
+        status,
+        headers: directResponse?.headers ?? nestedResponse?.headers,
+    };
+};
+
+export const getErrorStatus = (error: unknown): number | null =>
+    getHttpErrorDetails(error).status;
+
+export const getErrorHeader = (error: unknown, name: string): string | undefined => {
+    const { headers } = getHttpErrorDetails(error);
+
+    if (headers instanceof Headers) {
+        return headers.get(name) ?? undefined;
     }
 
-    const nested = (err as { cause?: { response?: { status?: unknown } } }).cause?.response?.status;
-    if (typeof nested === 'number') {
-        return nested;
+    if (headers !== null && typeof headers === 'object') {
+        const normalizedName = name.toLowerCase();
+        const entry = Object.entries(headers).find(([key]) => key.toLowerCase() === normalizedName);
+        return typeof entry?.[1] === 'string' ? entry[1] : undefined;
     }
 
-    return null;
+    return undefined;
 };
 
 export const isUnauthorized = (err: unknown): boolean => getErrorStatus(err) === 401;