mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
test(sync): add comprehensive timeout handling tests
Add unit tests for timeout handling functionality: - SuperSync provider: AbortController cleanup, error conversion, logging - Sync wrapper: timeout error detection with various error patterns - Restore service: exponential backoff retry logic (2s, 4s delays) Tests validate: - Memory leak prevention (timeout cleanup) - AbortError → timeout message conversion - Network error classification for retries - Retry attempt counting (max 3 attempts) - Case-insensitive error detection All tests passing (143 total across 3 test suites)
This commit is contained in:
parent
ae40f0ba2e
commit
c29194c17c
3 changed files with 293 additions and 0 deletions
|
|
@ -31,6 +31,10 @@ describe('SuperSyncRestoreService', () => {
|
|||
|
||||
mockSnackService = jasmine.createSpyObj('SnackService', ['open']);
|
||||
|
||||
// Spy on console methods for retry logging tests
|
||||
spyOn(console, 'warn');
|
||||
spyOn(console, 'error');
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
SuperSyncRestoreService,
|
||||
|
|
@ -179,6 +183,117 @@ describe('SuperSyncRestoreService', () => {
|
|||
});
|
||||
}, 10000); // Increase timeout to 10s to allow for retries (2s + 4s + overhead)
|
||||
|
||||
it('should use exponential backoff for retry delays', async () => {
|
||||
jasmine.clock().install();
|
||||
|
||||
const error = new Error('Network timeout');
|
||||
mockProvider.getStateAtSeq.and.returnValue(Promise.reject(error));
|
||||
|
||||
const restorePromise = service.restoreToPoint(100);
|
||||
|
||||
// First attempt should happen immediately
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
expect(mockProvider.getStateAtSeq).toHaveBeenCalledTimes(1);
|
||||
expect(console.warn).toHaveBeenCalledWith(
|
||||
jasmine.stringContaining('Restore failed due to network error, retrying (1/2)'),
|
||||
error,
|
||||
);
|
||||
|
||||
// Second attempt should happen after 2 seconds
|
||||
jasmine.clock().tick(1999);
|
||||
await Promise.resolve();
|
||||
expect(mockProvider.getStateAtSeq).toHaveBeenCalledTimes(1); // Still only 1
|
||||
|
||||
jasmine.clock().tick(1); // Now 2000ms total
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
expect(mockProvider.getStateAtSeq).toHaveBeenCalledTimes(2);
|
||||
expect(console.warn).toHaveBeenCalledWith(
|
||||
jasmine.stringContaining('Restore failed due to network error, retrying (2/2)'),
|
||||
error,
|
||||
);
|
||||
|
||||
// Third attempt should happen after 4 more seconds (6s total)
|
||||
jasmine.clock().tick(3999);
|
||||
await Promise.resolve();
|
||||
expect(mockProvider.getStateAtSeq).toHaveBeenCalledTimes(2); // Still only 2
|
||||
|
||||
jasmine.clock().tick(1); // Now 4000ms more (6000ms total)
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
expect(mockProvider.getStateAtSeq).toHaveBeenCalledTimes(3);
|
||||
|
||||
// After 3rd failure, should give up
|
||||
expect(mockSnackService.open).toHaveBeenCalledWith({
|
||||
type: 'ERROR',
|
||||
msg: T.F.SYNC.S.RESTORE_ERROR,
|
||||
});
|
||||
|
||||
jasmine.clock().uninstall();
|
||||
|
||||
await expectAsync(restorePromise).toBeRejectedWith(error);
|
||||
}, 15000);
|
||||
|
||||
it('should not retry on non-network errors', async () => {
|
||||
const error = new Error('Validation failed'); // Not a network error
|
||||
mockProvider.getStateAtSeq.and.returnValue(Promise.reject(error));
|
||||
|
||||
await expectAsync(service.restoreToPoint(100)).toBeRejectedWith(error);
|
||||
|
||||
// Should only attempt once (no retries for non-network errors)
|
||||
expect(mockProvider.getStateAtSeq).toHaveBeenCalledTimes(1);
|
||||
expect(mockSnackService.open).toHaveBeenCalledWith({
|
||||
type: 'ERROR',
|
||||
msg: T.F.SYNC.S.RESTORE_ERROR,
|
||||
});
|
||||
});
|
||||
|
||||
it('should retry on "failed to fetch" errors', async () => {
|
||||
const error = new Error('Failed to fetch');
|
||||
mockProvider.getStateAtSeq.and.returnValue(Promise.reject(error));
|
||||
|
||||
await expectAsync(service.restoreToPoint(100)).toBeRejectedWith(error);
|
||||
|
||||
// Should have retried (failed to fetch is a network error)
|
||||
expect(mockProvider.getStateAtSeq).toHaveBeenCalledTimes(3);
|
||||
}, 10000);
|
||||
|
||||
it('should retry on 504 errors', async () => {
|
||||
const error = new Error('504 Gateway Timeout');
|
||||
mockProvider.getStateAtSeq.and.returnValue(Promise.reject(error));
|
||||
|
||||
await expectAsync(service.restoreToPoint(100)).toBeRejectedWith(error);
|
||||
|
||||
// Should have retried (504 is a network/timeout error)
|
||||
expect(mockProvider.getStateAtSeq).toHaveBeenCalledTimes(3);
|
||||
}, 10000);
|
||||
|
||||
it('should succeed on second attempt after one failure', async () => {
|
||||
let attemptCount = 0;
|
||||
mockProvider.getStateAtSeq.and.callFake(() => {
|
||||
attemptCount++;
|
||||
if (attemptCount === 1) {
|
||||
return Promise.reject(new Error('timeout'));
|
||||
}
|
||||
return Promise.resolve({
|
||||
state: mockState,
|
||||
serverSeq: 100,
|
||||
generatedAt: Date.now(),
|
||||
});
|
||||
});
|
||||
|
||||
await service.restoreToPoint(100);
|
||||
|
||||
// Should have attempted twice (failed once, succeeded on retry)
|
||||
expect(mockProvider.getStateAtSeq).toHaveBeenCalledTimes(2);
|
||||
expect(mockBackupService.importCompleteBackup).toHaveBeenCalledTimes(1);
|
||||
expect(mockSnackService.open).toHaveBeenCalledWith({
|
||||
type: 'SUCCESS',
|
||||
msg: T.F.SYNC.S.RESTORE_SUCCESS,
|
||||
});
|
||||
}, 10000);
|
||||
|
||||
it('should show error when import fails', async () => {
|
||||
const error = new Error('Import failed');
|
||||
mockBackupService.importCompleteBackup.and.returnValue(Promise.reject(error));
|
||||
|
|
|
|||
|
|
@ -1044,4 +1044,54 @@ describe('SyncWrapperService', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('_isTimeoutError', () => {
|
||||
it('should detect timeout keyword in error message', () => {
|
||||
const timeoutError = new Error('Request timeout after 75s');
|
||||
expect(service['_isTimeoutError'](timeoutError)).toBe(true);
|
||||
});
|
||||
|
||||
it('should detect 504 status code', () => {
|
||||
const error504 = new Error('504 Gateway Timeout');
|
||||
expect(service['_isTimeoutError'](error504)).toBe(true);
|
||||
});
|
||||
|
||||
it('should detect gateway timeout phrase', () => {
|
||||
const gatewayError = new Error('Error: gateway timeout from proxy');
|
||||
expect(service['_isTimeoutError'](gatewayError)).toBe(true);
|
||||
});
|
||||
|
||||
it('should be case insensitive', () => {
|
||||
const uppercaseError = new Error('REQUEST TIMEOUT');
|
||||
expect(service['_isTimeoutError'](uppercaseError)).toBe(true);
|
||||
|
||||
const mixedCaseError = new Error('Gateway TIMEOUT occurred');
|
||||
expect(service['_isTimeoutError'](mixedCaseError)).toBe(true);
|
||||
});
|
||||
|
||||
it('should not false-positive on network errors', () => {
|
||||
const networkError = new Error('Network error');
|
||||
expect(service['_isTimeoutError'](networkError)).toBe(false);
|
||||
});
|
||||
|
||||
it('should not false-positive on auth errors', () => {
|
||||
const authError = new Error('401 Unauthorized');
|
||||
expect(service['_isTimeoutError'](authError)).toBe(false);
|
||||
});
|
||||
|
||||
it('should not false-positive on generic errors', () => {
|
||||
const genericError = new Error('Something went wrong');
|
||||
expect(service['_isTimeoutError'](genericError)).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle non-Error objects', () => {
|
||||
expect(service['_isTimeoutError']('timeout string')).toBe(true);
|
||||
expect(service['_isTimeoutError']('regular error')).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle objects with toString()', () => {
|
||||
const errorObj = { toString: () => 'Error: timeout occurred' };
|
||||
expect(service['_isTimeoutError'](errorObj)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import {
|
|||
AuthFailSPError,
|
||||
} from '../../core/errors/sync-errors';
|
||||
import { SyncOperation } from '../provider.interface';
|
||||
import { SyncLog } from '../../../core/log';
|
||||
|
||||
// Helper to convert Blob to Uint8Array
|
||||
const blobToUint8Array = async (blob: Blob): Promise<Uint8Array> => {
|
||||
|
|
@ -1336,4 +1337,131 @@ describe('SuperSyncProvider', () => {
|
|||
expect(decompressedPayload.isPayloadEncrypted).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Request timeout handling', () => {
|
||||
beforeEach(() => {
|
||||
mockPrivateCfgStore.load.and.returnValue(Promise.resolve(testConfig));
|
||||
});
|
||||
|
||||
it('should clear timeout on successful response', async () => {
|
||||
const clearTimeoutSpy = spyOn(window, 'clearTimeout');
|
||||
|
||||
fetchSpy.and.returnValue(
|
||||
Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ ops: [], latestSeq: 100 }),
|
||||
} as Response),
|
||||
);
|
||||
|
||||
await provider.downloadOps(0, 'client-1');
|
||||
|
||||
// Verify timeout was cleared (prevents memory leaks)
|
||||
expect(clearTimeoutSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should clear timeout on error response', async () => {
|
||||
const clearTimeoutSpy = spyOn(window, 'clearTimeout');
|
||||
|
||||
fetchSpy.and.returnValue(
|
||||
Promise.resolve({
|
||||
ok: false,
|
||||
status: 500,
|
||||
statusText: 'Internal Server Error',
|
||||
text: () => Promise.resolve('Server error'),
|
||||
} as Response),
|
||||
);
|
||||
|
||||
await expectAsync(provider.downloadOps(0, 'client-1')).toBeRejected();
|
||||
|
||||
// Verify timeout was cleared even on error
|
||||
expect(clearTimeoutSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should pass AbortSignal to fetch', async () => {
|
||||
fetchSpy.and.returnValue(
|
||||
Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ ops: [], latestSeq: 0 }),
|
||||
} as Response),
|
||||
);
|
||||
|
||||
await provider.downloadOps(0, 'client-1');
|
||||
|
||||
const fetchCall = fetchSpy.calls.mostRecent();
|
||||
const options = fetchCall.args[1];
|
||||
expect(options.signal).toBeDefined();
|
||||
expect(options.signal instanceof AbortSignal).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle AbortError and convert to timeout error', async () => {
|
||||
const abortError = new DOMException('The operation was aborted', 'AbortError');
|
||||
fetchSpy.and.returnValue(Promise.reject(abortError));
|
||||
|
||||
try {
|
||||
await provider.downloadOps(0, 'client-1');
|
||||
fail('Should have thrown timeout error');
|
||||
} catch (error) {
|
||||
expect((error as Error).message).toContain('timeout after 75s');
|
||||
expect((error as Error).message).toContain('/api/sync/ops');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Performance logging', () => {
|
||||
let syncLogWarnSpy: jasmine.Spy;
|
||||
let syncLogErrorSpy: jasmine.Spy;
|
||||
|
||||
beforeEach(() => {
|
||||
mockPrivateCfgStore.load.and.returnValue(Promise.resolve(testConfig));
|
||||
syncLogWarnSpy = spyOn(SyncLog, 'warn');
|
||||
syncLogErrorSpy = spyOn(SyncLog, 'error');
|
||||
});
|
||||
|
||||
it('should not log warning for fast requests', async () => {
|
||||
fetchSpy.and.returnValue(
|
||||
Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ ops: [], latestSeq: 100 }),
|
||||
} as Response),
|
||||
);
|
||||
|
||||
await provider.downloadOps(0, 'client-1');
|
||||
|
||||
// Should not have logged warning for fast request
|
||||
expect(syncLogWarnSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should log error on fetch failure', async () => {
|
||||
fetchSpy.and.returnValue(Promise.reject(new Error('Network error')));
|
||||
|
||||
await expectAsync(provider.downloadOps(0, 'client-1')).toBeRejected();
|
||||
|
||||
// Should have logged error
|
||||
expect(syncLogErrorSpy).toHaveBeenCalledWith(
|
||||
'SuperSyncProvider',
|
||||
'SuperSync request failed',
|
||||
jasmine.objectContaining({
|
||||
path: '/api/sync/ops?sinceSeq=0&excludeClient=client-1',
|
||||
error: 'Network error',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should log timeout errors with path information', async () => {
|
||||
const abortError = new DOMException('The operation was aborted', 'AbortError');
|
||||
fetchSpy.and.returnValue(Promise.reject(abortError));
|
||||
|
||||
await expectAsync(provider.downloadOps(0, 'client-1')).toBeRejected();
|
||||
|
||||
// Should have logged timeout error with path
|
||||
expect(syncLogErrorSpy).toHaveBeenCalledWith(
|
||||
'SuperSyncProvider',
|
||||
'SuperSync request timeout',
|
||||
jasmine.objectContaining({
|
||||
path: '/api/sync/ops?sinceSeq=0&excludeClient=client-1',
|
||||
timeoutMs: 75000,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue