Skip to content

Commit

Permalink
[federation] Event Emitter fails to call all listener (#6267)
Browse files Browse the repository at this point in the history
  • Loading branch information
EmrysMyrddin committed Jun 18, 2024
1 parent 00cfdac commit d5dd794
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 16 deletions.
5 changes: 5 additions & 0 deletions .changeset/flat-dragons-sparkle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-tools/federation': minor
---

Add `delayInSeconds` to the `failure` event to give users more control on failure handling.
6 changes: 6 additions & 0 deletions .changeset/gentle-snakes-worry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@graphql-tools/federation': minor
---

Add a the ability to start polling with a delay. This ease the handling of failure handling,
allowing to restart the manager and respecting GraphOS minimum retry delay.
6 changes: 6 additions & 0 deletions .changeset/real-seals-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@graphql-tools/federation': patch
---

Fix Supergraph Manager Event Emitter not calling every listener when at least one has been
registered using `once` method.
35 changes: 22 additions & 13 deletions packages/federation/src/managed-federation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ export type SupergraphSchemaManagerOptions = Omit<
export class SupergraphSchemaManager extends EventEmitter<{
schema: [GraphQLSchema];
error: [FetchError | unknown];
failure: [FetchError | unknown];
failure: [FetchError | unknown, number];
log: [{ source: 'uplink' | 'manager'; message: string; level: 'error' | 'warn' | 'info' }];
}> {
public schema?: GraphQLSchema = undefined;
Expand All @@ -323,27 +323,34 @@ export class SupergraphSchemaManager extends EventEmitter<{
});
}

start() {
this.#log('info', 'Polling started');
this.#fetchSchema();
}
start = (delayInSeconds = 0) => {
if (this.#timeout) {
this.stop();
}

forcePull() {
this.#fetchSchema();
this.#retries = 1;
this.#timeout = setTimeout(() => {
this.#log('info', 'Polling started');
this.#retries = 1;
this.#fetchSchema();
}, delayInSeconds * 1000);
};

forcePull = () => {
if (this.#timeout) {
clearTimeout(this.#timeout);
this.#timeout = undefined;
}
}
this.#retries = 1;
this.#fetchSchema();
};

stop() {
stop = () => {
this.#log('info', 'Polling stopped');
if (this.#timeout) {
clearTimeout(this.#timeout);
this.#timeout = undefined;
}
}
};

#fetchSchema = async () => {
const { retryDelaySeconds = 0, minDelaySeconds = 0 } = this.options;
Expand Down Expand Up @@ -387,11 +394,13 @@ export class SupergraphSchemaManager extends EventEmitter<{

#retryOnError = (error: unknown, delayInSeconds: number) => {
const { maxRetries = 3 } = this.options;
this.#log('error', 'Failed to pull schema from managed federation:');
const message = (error as { message: string })?.message;
this.#log('error', `Failed to pull schema from managed federation: ${message}`);

if (this.#retries >= maxRetries) {
this.#timeout = undefined;
this.#log('error', 'Max retries reached, giving up');
this.emit('failure', error);
this.emit('failure', error, delayInSeconds);
return;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/federation/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ export class EventEmitter<T extends EventMap<T>> {
if (!listeners) {
return false;
}
for (const listener of listeners) {
listener(...args);
for (let i = listeners.length - 1; i >= 0; i--) {
listeners[i](...args);
}
return true;
}
Expand Down
15 changes: 15 additions & 0 deletions packages/federation/test/managed-federation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ describe('Managed Federation', () => {
afterEach(() => {
manager?.stop();
});

it('should allow to wait for initial schema load', async () => {
manager = new SupergraphSchemaManager({
fetch: mockSDL,
Expand Down Expand Up @@ -187,6 +188,20 @@ describe('Managed Federation', () => {
expect(onSchemaChange).toHaveBeenCalledTimes(3);
});

it('should call onSchemaChange for on and once listeners', async () => {
manager = new SupergraphSchemaManager({
fetch: mockSDL,
});

const onSchemaChange = jest.fn();
manager.once('schema', onSchemaChange);
manager.on('schema', onSchemaChange);
manager.start();

await delay(0.05);
expect(onSchemaChange).toHaveBeenCalledTimes(2);
});

it('should retry on exceptions', async () => {
manager = new SupergraphSchemaManager({
fetch: mockError,
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"outDir": "dist",
"baseUrl": ".",

"target": "es2021",
"target": "es2022",
"module": "esnext",
"moduleResolution": "node",
"lib": ["esnext"],
Expand Down

0 comments on commit d5dd794

Please sign in to comment.