Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/common/container-definitions/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ export interface IContainer extends IEventProvider<IContainerEvents> {
*
* By default, the container will close if attach fails.
* However, closure can now be avoided in most cased by setting:
* Fluid.Container.RetryOnAttachFailure to true
* Fluid.Container.DisableCloseOnAttachFailure to true
* via the config provider passed to the loader.
*
* If attach fails, check the closed property to discover if retry is possible.
Expand All @@ -431,7 +431,9 @@ export interface IContainer extends IEventProvider<IContainerEvents> {
*/
attach(
request: IRequest,
attachProps?: { deltaConnection?: "none" | "delayed" },
attachProps?: {
deltaConnection?: "none" | "delayed";
},
): Promise<void>;

/**
Expand Down
11 changes: 10 additions & 1 deletion packages/loader/container-loader/src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,9 @@ export class Container
});

// only enable the new behavior if the config is set
if (this.mc.config.getBoolean("Fluid.Container.RetryOnAttachFailure") !== true) {
if (
this.mc.config.getBoolean("Fluid.Container.DisableCloseOnAttachFailure") !== true
) {
attachP = attachP.catch((error) => {
throw normalizeErrorAndClose(error);
});
Expand Down Expand Up @@ -1547,6 +1549,12 @@ export class Container
service.on("metadataUpdate", this.metadataUpdateHandler);
}
} else {
// When DisableCloseOnAttachFailure is enabled, use no internal retries
// The consumer will own the retry policy
const disableCloseOnAttachFailure =
this.mc.config.getBoolean("Fluid.Container.DisableCloseOnAttachFailure") === true;
const maxRetries = disableCloseOnAttachFailure ? 0 : undefined;

service = await runWithRetry(
async () =>
this.serviceFactory.createContainer(
Expand All @@ -1560,6 +1568,7 @@ export class Container
{
cancel: this._deltaManager.closeAbortController.signal,
}, // progress
maxRetries,
);
}
return service;
Expand Down
4 changes: 2 additions & 2 deletions packages/loader/container-loader/src/test/loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ describe("loader unit test", () => {
urlResolver: failProxy(),
configProvider: {
getRawConfig: (name): ConfigTypes =>
name === "Fluid.Container.RetryOnAttachFailure" ? true : undefined,
name === "Fluid.Container.DisableCloseOnAttachFailure" ? true : undefined,
},
});
const detached = await loader.createDetachedContainer({ package: "none" });
Expand Down Expand Up @@ -136,7 +136,7 @@ describe("loader unit test", () => {
}),
configProvider: {
getRawConfig: (name): ConfigTypes =>
name === "Fluid.Container.RetryOnAttachFailure" ? true : undefined,
name === "Fluid.Container.DisableCloseOnAttachFailure" ? true : undefined,
},
});
const detached = await loader.createDetachedContainer({ package: "none" });
Expand Down
31 changes: 31 additions & 0 deletions packages/loader/driver-utils/src/runWithRetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { delay } from "@fluidframework/core-utils/internal";
import { DriverErrorTypes } from "@fluidframework/driver-definitions/internal";
import {
isFluidError,
wrapError,
type ITelemetryLoggerExt,
} from "@fluidframework/telemetry-utils/internal";

Expand Down Expand Up @@ -57,6 +58,7 @@ export async function runWithRetry<T>(
fetchCallName: string,
logger: ITelemetryLoggerExt,
progress: IProgress,
maxRetries?: number,
): Promise<T> {
let result: T | undefined;
let success = false;
Expand Down Expand Up @@ -122,6 +124,35 @@ export async function runWithRetry<T>(
}

numRetries++;

// Check if max retries limit has been reached
if (maxRetries !== undefined && numRetries > maxRetries) {
logger.sendTelemetryEvent(
Comment on lines +128 to +130
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

progress.maxRetries is used without validation. If a caller passes NaN, Infinity, a negative value, or a non-integer, the comparison numRetries > progress.maxRetries can behave unexpectedly (e.g., NaN makes the limit never trigger, reintroducing infinite retries). Consider normalizing the option (e.g., require a finite, non-negative integer; otherwise treat as undefined or throw a non-retriable UsageError).

Copilot uses AI. Check for mistakes.
{
eventName: `${fetchCallName}_maxRetriesExceeded`,
retry: numRetries - 1,
maxRetries,
duration: performanceNow() - startTime,
fetchCallName,
},
error,
);
// Wrap the original error to preserve its details while marking it non-retriable
throw wrapError(
error,
(message) =>
new NonRetryableError(
`runWithRetry failed after max retries: ${message}`,
DriverErrorTypes.genericError,
{
driverVersion: pkgVersion,
fetchCallName,
maxRetries,
},
),
);
}

lastError = error;
// Wait for the calculated time before retrying.
retryAfterMs = calculateMaxWaitTime(retryAfterMs, error);
Expand Down
96 changes: 96 additions & 0 deletions packages/loader/driver-utils/src/test/runWithRetry.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,100 @@ describe("runWithRetry Tests", () => {
assert.strictEqual((error as any).reason, "Sample abort reason");
}
});

it("Should stop retrying after maxRetries is exceeded", async () => {
let retryTimes = 0;
const api = async (): Promise<boolean> => {
retryTimes += 1;
const error = new Error("Throw error");
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access -- TODO: use a real type
(error as any).canRetry = true;
throw error;
};

try {
await runWithFastSetTimeout(async () => runWithRetry(api, "test", logger, {}, 3));
assert.fail("Should not succeed");
} catch (error) {
// Verify the wrapped error includes the original error message
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment -- TODO: use a real type
const errorMessage = (error as any).message;
assert.strictEqual(errorMessage, "runWithRetry failed after max retries: Throw error");
// Verify the original error is preserved in the cause property
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment -- TODO: use a real type
const causeMessage = (error as any).cause?.message;
assert.strictEqual(causeMessage, "Throw error");
}
// Initial call + maxRetries attempts
assert.strictEqual(retryTimes, 4, "Should retry exactly maxRetries times");
});

it("Should succeed before maxRetries is exceeded", async () => {
let retryTimes = 0;
const api = async (): Promise<boolean> => {
retryTimes += 1;
// Succeed on the 3rd attempt (after 2 failures)
if (retryTimes < 3) {
const error = new Error("Throw error");
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access -- TODO: use a real type
(error as any).canRetry = true;
throw error;
}
return true;
};

const success = await runWithFastSetTimeout(async () =>
runWithRetry(api, "test", logger, {}, 5),
);
assert.strictEqual(success, true, "Should succeed");
assert.strictEqual(retryTimes, 3, "Should take 3 attempts to succeed");
});

it("Should retry infinitely when maxRetries is undefined", async () => {
const totalRetries = 10;
let retryTimes = 0;
const api = async (): Promise<boolean> => {
retryTimes += 1;
if (retryTimes <= totalRetries) {
const error = new Error("Throw error");
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access -- TODO: use a real type
(error as any).canRetry = true;
throw error;
}
return true;
};

const success = await runWithFastSetTimeout(async () =>
runWithRetry(api, "test", logger, {}),
);
assert.strictEqual(success, true, "Should succeed");
assert.strictEqual(retryTimes, totalRetries + 1, "Should retry until success");
});

it("Should fail immediately with maxRetries set to 0", async () => {
let retryTimes = 0;
const api = async (): Promise<boolean> => {
retryTimes += 1;
const error = new Error("Throw error");
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access -- TODO: use a real type
(error as any).canRetry = true;
throw error;
};

try {
await runWithFastSetTimeout(async () => runWithRetry(api, "test", logger, {}, 0));
assert.fail("Should not succeed");
} catch (error) {
// Verify the wrapped error includes the original error message
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment -- TODO: use a real type
const errorMessage = (error as any).message;
assert.strictEqual(errorMessage, "runWithRetry failed after max retries: Throw error");
// Verify the original error is preserved in the cause property
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment -- TODO: use a real type
const causeMessage = (error as any).cause?.message;
assert.strictEqual(causeMessage, "Throw error");
}
// Only the initial call, no retries
assert.strictEqual(retryTimes, 1, "Should not retry at all");
});
});
2 changes: 1 addition & 1 deletion packages/test/test-end-to-end-tests/src/test/blobs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ function serializationTests({
loaderProps: {
documentServiceFactory,
configProvider: createTestConfigProvider({
"Fluid.Container.RetryOnAttachFailure": true,
"Fluid.Container.DisableCloseOnAttachFailure": true,
}),
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -956,7 +956,7 @@ describeCompat("Detached Container", "NoCompat", (getTestObjectProvider, apis) =
{
configProvider: {
getRawConfig: (name) =>
name === "Fluid.Container.RetryOnAttachFailure" ? true : undefined,
name === "Fluid.Container.DisableCloseOnAttachFailure" ? true : undefined,
},
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describeCompat(
logger: provider.logger,
configProvider: {
getRawConfig: (name) =>
name === "Fluid.Container.RetryOnAttachFailure" ? true : undefined,
name === "Fluid.Container.DisableCloseOnAttachFailure" ? true : undefined,
},
});
return testLoader;
Expand Down
Loading