Skip to content

Commit

Permalink
feat: Add retry and retryDelay Properties to Enhance Download Met…
Browse files Browse the repository at this point in the history
…hod Reliability (#734)

* feat: Add retry and retryDelay

* chore: remove retryDelay and retry props from NormalizedScriptLocator

* chore:  change waitMs to inline function

* chore: throw an error on non-network errors immediately

* chore: make loadScriptWithRetry protected

* chore: fix typecheck errors

* chore: add changeset
  • Loading branch information
hexboy authored Oct 15, 2024
1 parent 206d76f commit b455503
Show file tree
Hide file tree
Showing 5 changed files with 282 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/tidy-eels-clean.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@callstack/repack": minor
---

Add a mechanism for retrying downloads of scripts through `retry` and `retryDelay` properties
7 changes: 6 additions & 1 deletion packages/repack/src/modules/ScriptManager/Script.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ export class Script {
url: locator.url,
absolute: locator.absolute ?? false,
timeout: locator.timeout ?? Script.DEFAULT_TIMEOUT,
retry: locator.retry,
retryDelay: locator.retryDelay,
query: new URLSearchParams(locator.query).toString() || undefined,
body,
headers: Object.keys(headers).length ? headers : undefined,
Expand All @@ -150,7 +152,10 @@ export class Script {
constructor(
public readonly scriptId: string,
public readonly caller: string | undefined,
public readonly locator: NormalizedScriptLocator,
public readonly locator: NormalizedScriptLocator & {
retry?: number;
retryDelay?: number;
},
public readonly cache: boolean = true
) {}

Expand Down
48 changes: 47 additions & 1 deletion packages/repack/src/modules/ScriptManager/ScriptManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ const CACHE_ENV = __DEV__ ? 'debug' : 'release';

const CACHE_KEY = [CACHE_NAME, CACHE_VERSION, CACHE_ENV].join('.');

const LOADING_ERROR_CODES = [
// android
'NetworkFailure',
'RequestFailure',
// ios
'ScriptDownloadFailure',
];

/* Options for resolver when adding it to a `ScriptManager`. */
export interface ResolverOptions {
/**
Expand Down Expand Up @@ -316,7 +324,7 @@ export class ScriptManager extends EventEmitter {

try {
this.emit('loading', script.toObject());
await this.nativeScriptManager.loadScript(scriptId, script.locator);
await this.loadScriptWithRetry(scriptId, script.locator);
this.emit('loaded', script.toObject());
} catch (error) {
const { code } = error as Error & { code: string };
Expand All @@ -329,6 +337,44 @@ export class ScriptManager extends EventEmitter {
}
}

/**
* Loads a script with retry logic.
*
* This function attempts to load a script using the nativeScriptManager.
* If the initial attempt fails, it retries the specified number of times
* with an optional delay between retries.
*
* @param {string} scriptId - The ID of the script to load.
* @param {NormalizedScriptLocator} locator - An NormalizedScriptLocator containing retry configuration.
* @param {number} [locator.retry=0] - The number of retry attempts.
* @param {number} [locator.retryDelay=0] - The delay in milliseconds between retries.
* @throws {Error} Throws an error if all retry attempts fail.
*/
protected async loadScriptWithRetry(
scriptId: string,
locator: NormalizedScriptLocator & { retryDelay?: number; retry?: number }
) {
const { retry = 0, retryDelay = 0 } = locator;
let attempts = retry + 1; // Include the initial attempt

while (attempts > 0) {
try {
await this.nativeScriptManager.loadScript(scriptId, locator);
return; // Successfully loaded the script, exit the loop
} catch (error) {
attempts--;
const { code } = error as Error & { code: string };
if (attempts > 0 && LOADING_ERROR_CODES.includes(code)) {
if (retryDelay > 0) {
await new Promise((resolve) => setTimeout(resolve, retryDelay));
}
} else {
throw error; // No more retries, throw the error
}
}
}
}

/**
* Resolves given script's location and downloads it without executing.
* This function can be awaited to detect if the script was downloaded and for error handling.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import NativeScriptManager from '../NativeScriptManager';
import { Script } from '../Script';
import { ScriptManager } from '../ScriptManager';

Expand Down Expand Up @@ -42,6 +43,15 @@ class FakeCache {
}
}

class ScriptLoaderError extends Error {
code: string;

constructor(message: string, code: string) {
super(message);
this.code = code;
}
}

beforeEach(() => {
ScriptManager.init();
});
Expand Down Expand Up @@ -532,4 +542,198 @@ describe('ScriptManagerAPI', () => {
);
expect(script6.locator.fetch).toBe(true);
});

it('should throw an error on non-network errors occurrence in load script with retry', async () => {
const cache = new FakeCache();
ScriptManager.shared.setStorage(cache);
ScriptManager.shared.addResolver(async (scriptId, caller) => {
expect(caller).toEqual('appB');

return {
url: Script.getRemoteURL(`http://domain.ext/${scriptId}`),
retry: 2,
retryDelay: 100,
};
});

const scriptId = 'src_App_js';
const script = await ScriptManager.shared.resolveScript(scriptId, 'appB');
expect(script.locator).toEqual({
url: 'http://domain.ext/src_App_js.chunk.bundle',
fetch: true,
absolute: false,
method: 'GET',
timeout: Script.DEFAULT_TIMEOUT,
verifyScriptSignature: 'off',
uniqueId: 'appB_' + scriptId,
retry: 2,
retryDelay: 100,
});

jest.useFakeTimers({ advanceTimers: true });
jest.spyOn(global, 'setTimeout');

// Mock the nativeScriptManager.loadScript to fail immediately on non network error
jest
.mocked(NativeScriptManager.loadScript)
.mockRejectedValueOnce(
new ScriptLoaderError('First attempt failed', 'ScriptEvalFailure')
);

await expect(
ScriptManager.shared.loadScript(scriptId, 'appB')
).rejects.toThrow('First attempt failed');

expect(setTimeout).toHaveBeenCalledTimes(0);
expect(NativeScriptManager.loadScript).toHaveBeenCalledTimes(1);
expect(NativeScriptManager.loadScript).toHaveBeenCalledWith(scriptId, {
absolute: false,
fetch: false,
method: 'GET',
retry: 2,
retryDelay: 100,
timeout: 30000,
uniqueId: 'appB_' + scriptId,
url: 'http://domain.ext/src_App_js.chunk.bundle',
verifyScriptSignature: 'off',
});
jest.useRealTimers();
});

it('should load script with retry', async () => {
const cache = new FakeCache();
ScriptManager.shared.setStorage(cache);
ScriptManager.shared.addResolver(async (scriptId, caller) => {
expect(caller).toEqual('main');

return {
url: Script.getRemoteURL(`http://domain.ext/${scriptId}`),
retry: 2,
retryDelay: 100,
};
});

const scriptId = 'src_App_js';
const script = await ScriptManager.shared.resolveScript(scriptId, 'main');
expect(script.locator).toEqual({
url: 'http://domain.ext/src_App_js.chunk.bundle',
fetch: true,
absolute: false,
method: 'GET',
timeout: Script.DEFAULT_TIMEOUT,
verifyScriptSignature: 'off',
uniqueId: 'main_src_App_js',
retry: 2,
retryDelay: 100,
});

// Mock the nativeScriptManager.loadScript to fail twice and succeed on the third attempt
jest
.mocked(NativeScriptManager.loadScript)
.mockRejectedValueOnce(
new ScriptLoaderError('First attempt failed', 'RequestFailure')
)
.mockRejectedValueOnce(
new ScriptLoaderError('Second attempt failed', 'RequestFailure')
)
.mockResolvedValueOnce(null);

jest.useFakeTimers({ advanceTimers: true });
await ScriptManager.shared.loadScript(scriptId, 'main');

expect(NativeScriptManager.loadScript).toHaveBeenCalledTimes(3);
expect(NativeScriptManager.loadScript).toHaveBeenCalledWith(scriptId, {
absolute: false,
fetch: false,
method: 'GET',
retry: 2,
retryDelay: 100,
timeout: 30000,
uniqueId: 'main_src_App_js',
url: 'http://domain.ext/src_App_js.chunk.bundle',
verifyScriptSignature: 'off',
});
jest.useRealTimers();
});

it('should throw error if all retry attempts fail', async () => {
const cache = new FakeCache();
ScriptManager.shared.setStorage(cache);
ScriptManager.shared.addResolver(async (scriptId) => {
return {
url: Script.getRemoteURL(`http://domain.ext/${scriptId}`),
retry: 2,
fetch: false,
retryDelay: 100,
};
});

const scriptId = 'src_App_js';
// Mock the nativeScriptManager.loadScript to fail all attempts
jest
.mocked(NativeScriptManager.loadScript)
.mockRejectedValueOnce(
new ScriptLoaderError('First attempt failed', 'NetworkFailure')
)
.mockRejectedValueOnce(
new ScriptLoaderError('Second attempt failed', 'NetworkFailure')
)
.mockRejectedValueOnce(
new ScriptLoaderError('Third attempt failed', 'NetworkFailure')
);

jest.useFakeTimers({ advanceTimers: true });
await expect(ScriptManager.shared.loadScript(scriptId)).rejects.toThrow(
'Third attempt failed'
);

expect(NativeScriptManager.loadScript).toHaveBeenCalledTimes(3);
expect(NativeScriptManager.loadScript).toHaveBeenCalledWith(scriptId, {
absolute: false,
fetch: true,
method: 'GET',
retry: 2,
retryDelay: 100,
timeout: 30000,
uniqueId: scriptId,
url: 'http://domain.ext/src_App_js.chunk.bundle',
verifyScriptSignature: 'off',
});
jest.useRealTimers();
});

it('should retry with delay', async () => {
const cache = new FakeCache();
ScriptManager.shared.setStorage(cache);
ScriptManager.shared.addResolver(async (scriptId) => {
return {
url: Script.getRemoteURL(`http://domain.ext/${scriptId}`),
retry: 2,
retryDelay: 100,
};
});

const scriptId = 'src_App_js';
// Mock the nativeScriptManager.loadScript to fail twice and succeed on the third attempt
jest
.mocked(NativeScriptManager.loadScript)
.mockRejectedValueOnce(
new ScriptLoaderError('First attempt failed', 'ScriptDownloadFailure')
)
.mockRejectedValueOnce(
new ScriptLoaderError('Second attempt failed', 'ScriptDownloadFailure')
)
.mockResolvedValueOnce(null);

jest.useFakeTimers({ advanceTimers: true });
jest.spyOn(global, 'setTimeout');

const loadScriptPromise = ScriptManager.shared.loadScript(scriptId);

await expect(loadScriptPromise).resolves.toBeUndefined();

expect(setTimeout).toHaveBeenCalledTimes(2);
expect(NativeScriptManager.loadScript).toHaveBeenCalledTimes(3);
jest.useRealTimers();
});
});
20 changes: 20 additions & 0 deletions packages/repack/src/modules/ScriptManager/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,26 @@ export interface ScriptLocator {
*/
timeout?: number;

/**
* Number of times to retry fetching the script in case of failure.
*
* If the script fails to download due to network issues or server errors,
* this field determines how many additional attempts should be made to fetch it.
* A value of `0` means no retries will be attempted.
* Defaults to `0` if not specified.
*/
retry?: number;

/**
* Delay in milliseconds between each retry attempt.
*
* This field specifies the wait time between consecutive retry attempts
* if the script download fails. It helps to avoid immediate retries and allows
* the network or server to recover before trying again.
* Defaults to `0` if not specified.
*/
retryDelay?: number;

/**
* Flag indicating whether the URL is an absolute FileSystem URL on a target device.
* Useful if you're using custom code to download the script and you want `ScriptManager` to
Expand Down

0 comments on commit b455503

Please sign in to comment.