Skip to content

feat: abort controller #183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 15, 2023
Merged

feat: abort controller #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a50458b
feat: abort controller
kwasniew Nov 14, 2023
deeed91
Merge branch 'main' of github.com:Unleash/unleash-proxy-client-js int…
kwasniew Nov 14, 2023
5d4db3a
feat: abort controller
kwasniew Nov 14, 2023
4823e5b
feat: abort controller
kwasniew Nov 14, 2023
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
19 changes: 19 additions & 0 deletions src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,25 @@ test('Should publish error when fetch fails', (done) => {
});
});

test('Should abort previous request', async () => {
fetchMock.mockResponse(JSON.stringify(data));
const abortSpy = jest.spyOn(AbortController.prototype, 'abort');

const config: IConfig = {
url: 'http://localhost/test',
clientKey: '12',
appName: 'web',
};
const client = new UnleashClient(config);
await client.start();
client.updateContext({ userId: '123' }); // abort 1
client.updateContext({ userId: '456' }); // abort 2
await client.updateContext({ userId: '789' });

expect(abortSpy).toBeCalledTimes(2);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also wanted to test error emitter on abort but the mocking fetch library does not support AbortController

abortSpy.mockRestore();
});

test.each([400, 401, 403, 404, 429, 500, 502, 503])(
'Should publish error when fetch receives a %d error',
async (errorCode) => {
Expand Down
34 changes: 34 additions & 0 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ interface IConfig extends IStaticContext {
storageProvider?: IStorageProvider;
context?: IMutableContext;
fetch?: any;
createAbortController?: () => AbortController;
bootstrap?: IToggle[];
bootstrapOverride?: boolean;
headerName?: string;
Expand Down Expand Up @@ -94,6 +95,18 @@ export const resolveFetch = () => {
return undefined;
};

const resolveAbortController = () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

similar pattern to how we handle fetch

try {
if (typeof window !== 'undefined' && 'AbortController' in window) {
return () => new window.AbortController();
} else if ('fetch' in globalThis) {
return () => new globalThis.AbortController();
}
} catch (e) {
console.error('Unleash failed to resolve "AbortController" factory', e);
Copy link
Member

Choose a reason for hiding this comment

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

This should basically never happen, right? This is if someone is doing something unusual like swapping out fetch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct - someone would need to take deliberate effort to end up in this situation

}
};

export class UnleashClient extends TinyEmitter {
private toggles: IToggle[] = [];
private impressionDataAll: boolean;
Expand All @@ -107,6 +120,8 @@ export class UnleashClient extends TinyEmitter {
private metrics: Metrics;
private ready: Promise<void>;
private fetch: any;
private createAbortController?: () => AbortController;
private abortController?: AbortController | null;
private bootstrap?: IToggle[];
private bootstrapOverride: boolean;
private headerName: string;
Expand All @@ -128,6 +143,7 @@ export class UnleashClient extends TinyEmitter {
environment = 'default',
context,
fetch = resolveFetch(),
createAbortController = resolveAbortController(),
bootstrap,
bootstrapOverride = true,
headerName = 'Authorization',
Expand Down Expand Up @@ -176,8 +192,14 @@ export class UnleashClient extends TinyEmitter {
'Unleash: You must either provide your own "fetch" implementation or run in an environment where "fetch" is available.'
);
}
if (!createAbortController) {
console.error(
'Unleash: You must either provide your own "AbortController" implementation or run in an environment where "AbortController" is available.'
);
}

this.fetch = fetch;
this.createAbortController = createAbortController;
this.bootstrap =
bootstrap && bootstrap.length > 0 ? bootstrap : undefined;
this.bootstrapOverride = bootstrapOverride;
Expand Down Expand Up @@ -366,6 +388,15 @@ export class UnleashClient extends TinyEmitter {

private async fetchToggles() {
if (this.fetch) {
if (this.abortController) {
this.abortController.abort();
}
this.abortController =
this.createAbortController && this.createAbortController();
const signal = this.abortController
? this.abortController.signal
: undefined;

try {
const isPOST = this.usePOSTrequests;

Expand All @@ -382,6 +413,7 @@ export class UnleashClient extends TinyEmitter {
cache: 'no-cache',
headers: this.getHeaders(),
body,
signal,
});
if (response.ok && response.status !== 304) {
this.etag = response.headers.get('ETag') || '';
Expand All @@ -404,6 +436,8 @@ export class UnleashClient extends TinyEmitter {
} catch (e) {
console.error('Unleash: unable to fetch feature toggles', e);
this.emit(EVENTS.ERROR, e);
} finally {
this.abortController = null;
}
}
}
Expand Down