diff --git a/packages/drivers/routerlicious-driver/src/restWrapper.ts b/packages/drivers/routerlicious-driver/src/restWrapper.ts index b906db3a2964..a7d38849d3fc 100644 --- a/packages/drivers/routerlicious-driver/src/restWrapper.ts +++ b/packages/drivers/routerlicious-driver/src/restWrapper.ts @@ -5,7 +5,7 @@ import { performanceNow } from "@fluid-internal/client-utils"; import type { ITelemetryBaseProperties } from "@fluidframework/core-interfaces"; -import { assert } from "@fluidframework/core-utils/internal"; +import { assert, delay } from "@fluidframework/core-utils/internal"; import type { RateLimiter } from "@fluidframework/driver-utils/internal"; import { GenericNetworkError, NonRetryableError } from "@fluidframework/driver-utils/internal"; import { @@ -48,6 +48,9 @@ const buildRequestInitConfig = (requestConfig: RequestConfig): RequestInit => { return requestInit; }; +// Transport-level failures that are safe to retry on a fresh socket. +const transientNetworkErrorPattern = /socket hang up|ECONNRESET|EPIPE/i; + export interface IR11sResponse { content: T; headers: Map; @@ -117,6 +120,14 @@ class RouterliciousRestWrapper extends RestWrapper { */ private readonly retryCounter = new Map(); + /** + * Maximum number of attempts (1 initial + retries) for a single fetch that fails with a transient network error. + */ + private static readonly maxNetworkErrorAttempts = 3; + + /** Delay between transient network-error retries. */ + private static readonly networkErrorRetryDelayMs = 250; + constructor( logger: TelemetryLoggerExt, private readonly rateLimiter: RateLimiter, @@ -169,9 +180,17 @@ class RouterliciousRestWrapper extends RestWrapper { const fetchRequestConfig = buildRequestInitConfig(translatedConfig); const res = await this.rateLimiter.schedule(async () => { - const perfStart = performanceNow(); - const result = await fetch(completeRequestUrl, fetchRequestConfig).catch( - async (error) => { + // Retry transient transport failures (e.g. a pooled socket closed by the peer while + // this process's execution context was frozen) on a fresh socket before surfacing the error. + for (let attempt = 1; ; attempt++) { + const perfStart = performanceNow(); + try { + const result = await fetch(completeRequestUrl, fetchRequestConfig); + return { + response: result, + duration: performanceNow() - perfStart, + }; + } catch (error: any) { // on failure, add the request entry into the retryCounter map to count the subsequent retries, if any this.retryCounter.set(requestKey, requestRetryCount ? requestRetryCount + 1 : 1); @@ -187,12 +206,34 @@ class RouterliciousRestWrapper extends RestWrapper { const errorMessage = isNetworkError ? `NetworkError: ${error.message}` : safeStringify(error); + + // A self-signed-cert failure is permanent, never transient — exclude it from retry. + const isSelfSignedCertError = errorMessage.includes( + "failed, reason: self signed certificate", + ); + const errorCode: unknown = error?.code ?? error?.cause?.code; + const isTransientNetworkError = + !isSelfSignedCertError && + (isNetworkError || + (typeof errorCode === "string" && + transientNetworkErrorPattern.test(errorCode)) || + transientNetworkErrorPattern.test(error?.message ?? "") || + transientNetworkErrorPattern.test(error?.cause?.message ?? "")); + if ( + isTransientNetworkError && + canRetry && + attempt < RouterliciousRestWrapper.maxNetworkErrorAttempts + ) { + await delay(RouterliciousRestWrapper.networkErrorRetryDelayMs); + continue; + } + // If a service is temporarily down or a browser resource limit is reached, RestWrapper will throw // a network error with no status code (e.g. err:ERR_CONN_REFUSED or err:ERR_FAILED) and // the error message will start with NetworkError as defined in restWrapper.ts // If there exists a self-signed SSL certificates error, throw a NonRetryableError // TODO: instead of relying on string matching, filter error based on the error code like we do for websocket connections - const err = errorMessage.includes("failed, reason: self signed certificate") + const err = isSelfSignedCertError ? new NonRetryableError( errorMessage, RouterliciousErrorTypes.sslCertError, @@ -204,12 +245,8 @@ class RouterliciousRestWrapper extends RestWrapper { telemetryProps, ); throw err; - }, - ); - return { - response: result, - duration: performanceNow() - perfStart, - }; + } + } }); const response = res.response; diff --git a/packages/drivers/routerlicious-driver/src/test/restWrapper.spec.ts b/packages/drivers/routerlicious-driver/src/test/restWrapper.spec.ts index 3ac26aeee273..c38c1eca4d95 100644 --- a/packages/drivers/routerlicious-driver/src/test/restWrapper.spec.ts +++ b/packages/drivers/routerlicious-driver/src/test/restWrapper.spec.ts @@ -60,7 +60,7 @@ interface MockFetchEntry { /** Response body. Objects are JSON-serialized with content-type: application/json. */ body?: string | object; /** Simulate a network error by rejecting fetch with a TypeError */ - networkError?: { code: string }; + networkError?: { code?: string; message?: string }; /** Dynamically compute the response as [status, body] */ replyFn?: () => [number, string | object]; } @@ -138,8 +138,10 @@ function createMockFetch(entries: MockFetchEntry[]): MockFetch { // Simulate network error if (entry.networkError) { - const err = new TypeError(`request to ${inputUrl} failed`); - (err as any).code = entry.networkError.code; + const err = new TypeError(entry.networkError.message ?? `request to ${inputUrl} failed`); + if (entry.networkError.code !== undefined) { + (err as any).code = entry.networkError.code; + } throw err; } @@ -330,7 +332,9 @@ describe("RouterliciousDriverRestWrapper", () => { errorType: RouterliciousErrorTypes.fileNotFoundOrAccessDeniedError, }); }); - it("throws retriable error on Network Error", async () => { + it("throws retriable error on Network Error after exhausting retries", async () => { + // request() now retries transient network errors, so mock a failure for each of the + // 3 attempts (1 initial + 2 retries) before it gives up. mockFetch = createMockFetch([ { method: "GET", @@ -338,6 +342,18 @@ describe("RouterliciousDriverRestWrapper", () => { path: testPath, networkError: { code: "ECONNRESET" }, }, + { + method: "GET", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET" }, + }, + { + method: "GET", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET" }, + }, ]); await assert.rejects(restWrapper.get(testUrl), { canRetry: true, @@ -542,7 +558,9 @@ describe("RouterliciousDriverRestWrapper", () => { errorType: RouterliciousErrorTypes.fileNotFoundOrAccessDeniedError, }); }); - it("throws retriable error on Network Error", async () => { + it("throws retriable error on Network Error after exhausting retries", async () => { + // request() now retries transient network errors, so mock a failure for each of the + // 3 attempts (1 initial + 2 retries) before it gives up. mockFetch = createMockFetch([ { method: "POST", @@ -550,6 +568,18 @@ describe("RouterliciousDriverRestWrapper", () => { path: testPath, networkError: { code: "ECONNRESET" }, }, + { + method: "POST", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET" }, + }, + { + method: "POST", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET" }, + }, ]); await assert.rejects(restWrapper.post(testUrl, { test: "payload" }), { canRetry: true, @@ -661,7 +691,9 @@ describe("RouterliciousDriverRestWrapper", () => { errorType: RouterliciousErrorTypes.fileNotFoundOrAccessDeniedError, }); }); - it("throws retriable error on Network Error", async () => { + it("throws retriable error on Network Error after exhausting retries", async () => { + // request() now retries transient network errors, so mock a failure for each of the + // 3 attempts (1 initial + 2 retries) before it gives up. mockFetch = createMockFetch([ { method: "PATCH", @@ -669,6 +701,18 @@ describe("RouterliciousDriverRestWrapper", () => { path: testPath, networkError: { code: "ECONNRESET" }, }, + { + method: "PATCH", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET" }, + }, + { + method: "PATCH", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET" }, + }, ]); await assert.rejects(restWrapper.patch(testUrl, { test: "payload" }), { canRetry: true, @@ -780,7 +824,9 @@ describe("RouterliciousDriverRestWrapper", () => { errorType: RouterliciousErrorTypes.fileNotFoundOrAccessDeniedError, }); }); - it("throws retriable error on Network Error", async () => { + it("throws retriable error on Network Error after exhausting retries", async () => { + // request() now retries transient network errors, so mock a failure for each of the + // 3 attempts (1 initial + 2 retries) before it gives up. mockFetch = createMockFetch([ { method: "DELETE", @@ -788,6 +834,18 @@ describe("RouterliciousDriverRestWrapper", () => { path: testPath, networkError: { code: "ECONNRESET" }, }, + { + method: "DELETE", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET" }, + }, + { + method: "DELETE", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET" }, + }, ]); await assert.rejects(restWrapper.delete(testUrl), { canRetry: true, @@ -795,4 +853,85 @@ describe("RouterliciousDriverRestWrapper", () => { }); }); }); + + // Transport-level retry: the routerlicious driver runs in AWS Lambda, whose execution context + // is frozen between invocations. A keep-alive socket pooled by the underlying fetch transport + // can be closed by the peer (e.g. an ALB) during the freeze; the next request reuses the dead + // socket and rejects with "socket hang up" / ECONNRESET / EPIPE. request() retries such + // transient network errors (a fresh socket is opened on retry) up to a small bound before + // surfacing the error. + describe("transient network error retry", () => { + it("retries a transient network error and succeeds when a later attempt connects", async () => { + mockFetch = createMockFetch([ + { + method: "GET", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET", message: "socket hang up" }, + }, + { method: "GET", origin: testHost, path: testPath, status: 200 }, + ]); + await assert.doesNotReject(restWrapper.get(testUrl)); + }); + + it("retries POST requests too", async () => { + mockFetch = createMockFetch([ + { + method: "POST", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET", message: "socket hang up" }, + }, + { method: "POST", origin: testHost, path: testPath, status: 200 }, + ]); + await assert.doesNotReject(restWrapper.post(testUrl, { test: "payload" })); + }); + + it("gives up after 3 attempts and throws a retriable network error", async () => { + // Fail all 3 attempts (1 initial + 2 retries). afterEach's mockFetch.done() asserts + // exactly 3 fetch calls were made, proving the loop doesn't retry beyond the bound. + mockFetch = createMockFetch([ + { + method: "GET", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET", message: "socket hang up" }, + }, + { + method: "GET", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET", message: "socket hang up" }, + }, + { + method: "GET", + origin: testHost, + path: testPath, + networkError: { code: "ECONNRESET", message: "socket hang up" }, + }, + ]); + await assert.rejects(restWrapper.get(testUrl), { + canRetry: true, + errorType: RouterliciousErrorTypes.genericNetworkError, + }); + }); + + it("does not retry a self-signed certificate error (non-retriable)", async () => { + mockFetch = createMockFetch([ + { + method: "GET", + origin: testHost, + path: testPath, + networkError: { + message: + "request to https://localhost/ failed, reason: self signed certificate in certificate chain", + }, + }, + ]); + await assert.rejects(restWrapper.get(testUrl), { + canRetry: false, + errorType: RouterliciousErrorTypes.sslCertError, + }); + }); + }); });