From bbc4d7ef3913148ad78b72062cb668cd1a41e13f Mon Sep 17 00:00:00 2001 From: Dogan AY Date: Thu, 30 Jan 2025 15:59:45 +0100 Subject: [PATCH 1/9] fix(sse): detect buffering and warn user --- .../test/datasource.test.ts | 2 +- .../src/events-subscription/index.ts | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/datasource-mongoose/test/datasource.test.ts b/packages/datasource-mongoose/test/datasource.test.ts index 1ea6298b07..bcce11279e 100644 --- a/packages/datasource-mongoose/test/datasource.test.ts +++ b/packages/datasource-mongoose/test/datasource.test.ts @@ -1,4 +1,4 @@ -import mongoose, { Schema } from 'mongoose'; +import mongoose, { Schema, mongo } from 'mongoose'; import * as CollectionModule from '../src/collection'; import MongooseDatasource from '../src/datasource'; diff --git a/packages/forestadmin-client/src/events-subscription/index.ts b/packages/forestadmin-client/src/events-subscription/index.ts index e8e1484c30..45533c9f56 100644 --- a/packages/forestadmin-client/src/events-subscription/index.ts +++ b/packages/forestadmin-client/src/events-subscription/index.ts @@ -10,12 +10,28 @@ import { ForestAdminClientOptionsWithDefaults } from '../types'; export default class EventsSubscriptionService implements BaseEventsSubscriptionService { private eventSource: EventSource; + private heartBeatTimeout: NodeJS.Timeout; constructor( private readonly options: ForestAdminClientOptionsWithDefaults, private readonly refreshEventsHandlerService: RefreshEventsHandlerService, ) {} + private async detectBuffering() { + clearTimeout(this.heartBeatTimeout); + + this.heartBeatTimeout = setTimeout(() => { + this.options.logger( + 'Error', + `Unable to detect ServerSentEvents Heartbeat. + Forest Admin uses ServerSentEvents to ensure that permission cache is up to date. + It seems that your agent does not receive events from our server, this may due to buffering of events from your reverse proxy. + https://docs.forestadmin.com/developer-guide-agents-nodejs/getting-started/install/troubleshooting + `, + ); + }, 45000); + } + async subscribeEvents(): Promise { if (!this.options.instantCacheRefresh) { this.options.logger( @@ -48,6 +64,12 @@ export default class EventsSubscriptionService implements BaseEventsSubscription eventSource.addEventListener('open', () => this.onEventOpenAgain()), ); + eventSource.on('heartbeat', () => { + clearTimeout(this.heartBeatTimeout); + }); + + this.detectBuffering(); + eventSource.addEventListener(ServerEventType.RefreshUsers, async () => this.refreshEventsHandlerService.refreshUsers(), ); From 35cca4b420c0b8e1e7fdd37993e4c9ba6f68089b Mon Sep 17 00:00:00 2001 From: Dogan AY Date: Thu, 30 Jan 2025 16:13:20 +0100 Subject: [PATCH 2/9] fix: lint --- packages/datasource-mongoose/test/datasource.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/datasource-mongoose/test/datasource.test.ts b/packages/datasource-mongoose/test/datasource.test.ts index bcce11279e..1ea6298b07 100644 --- a/packages/datasource-mongoose/test/datasource.test.ts +++ b/packages/datasource-mongoose/test/datasource.test.ts @@ -1,4 +1,4 @@ -import mongoose, { Schema, mongo } from 'mongoose'; +import mongoose, { Schema } from 'mongoose'; import * as CollectionModule from '../src/collection'; import MongooseDatasource from '../src/datasource'; From c620a0de3c9576a16474eda9734d40636420a658 Mon Sep 17 00:00:00 2001 From: Dogan AY Date: Thu, 30 Jan 2025 16:35:06 +0100 Subject: [PATCH 3/9] fix: some tests --- .../src/events-subscription/index.ts | 19 +++++++++++++------ .../test/events-subscription/index.test.ts | 6 +++++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/forestadmin-client/src/events-subscription/index.ts b/packages/forestadmin-client/src/events-subscription/index.ts index 45533c9f56..910fea0f9b 100644 --- a/packages/forestadmin-client/src/events-subscription/index.ts +++ b/packages/forestadmin-client/src/events-subscription/index.ts @@ -17,7 +17,7 @@ export default class EventsSubscriptionService implements BaseEventsSubscription private readonly refreshEventsHandlerService: RefreshEventsHandlerService, ) {} - private async detectBuffering() { + private detectBuffering() { clearTimeout(this.heartBeatTimeout); this.heartBeatTimeout = setTimeout(() => { @@ -60,13 +60,19 @@ export default class EventsSubscriptionService implements BaseEventsSubscription eventSource.addEventListener('error', this.onEventError.bind(this)); // Only listen after first open - eventSource.once('open', () => - eventSource.addEventListener('open', () => this.onEventOpenAgain()), + eventSource.addEventListener( + 'open', + () => eventSource.addEventListener('open', () => this.onEventOpenAgain()), + { once: true }, ); - eventSource.on('heartbeat', () => { - clearTimeout(this.heartBeatTimeout); - }); + eventSource.addEventListener( + 'heartbeat', + () => { + clearTimeout(this.heartBeatTimeout); + }, + { once: true }, + ); this.detectBuffering(); @@ -93,6 +99,7 @@ export default class EventsSubscriptionService implements BaseEventsSubscription * Close the current EventSource */ public close() { + clearTimeout(this.heartBeatTimeout); this.eventSource?.close(); } diff --git a/packages/forestadmin-client/test/events-subscription/index.test.ts b/packages/forestadmin-client/test/events-subscription/index.test.ts index 1ca0ea51e3..9ae9586682 100644 --- a/packages/forestadmin-client/test/events-subscription/index.test.ts +++ b/packages/forestadmin-client/test/events-subscription/index.test.ts @@ -35,7 +35,11 @@ describe('EventsSubscriptionService', () => { eventsSubscriptionService.subscribeEvents(); expect(addEventListener).toHaveBeenCalledWith('error', expect.any(Function)); - expect(once).toHaveBeenCalledWith('open', expect.any(Function)); + expect(addEventListener).toHaveBeenCalledWith('open', expect.any(Function), { once: true }); + + expect(addEventListener).toHaveBeenCalledWith('heartbeat', expect.any(Function), { + once: true, + }); expect(addEventListener).toHaveBeenCalledWith( ServerEventType.RefreshUsers, From 03363915aaca385b3d47dc74d98998d74e7916a2 Mon Sep 17 00:00:00 2001 From: Dogan AY Date: Thu, 30 Jan 2025 16:40:42 +0100 Subject: [PATCH 4/9] chore: update github action version --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d8334f5ff3..284d4cdbac 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -153,7 +153,7 @@ jobs: - name: Build doc run: yarn docs - name: Archive documentation artifacts - uses: actions/upload-pages-artifact@v1 + uses: actions/upload-pages-artifact@v4 with: path: api-reference From d5836f185da38c07cb0d9d7bb58f5f9cade80543 Mon Sep 17 00:00:00 2001 From: Dogan AY Date: Thu, 30 Jan 2025 16:44:27 +0100 Subject: [PATCH 5/9] fix: github action version --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 284d4cdbac..2c540e4760 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -153,7 +153,7 @@ jobs: - name: Build doc run: yarn docs - name: Archive documentation artifacts - uses: actions/upload-pages-artifact@v4 + uses: actions/upload-pages-artifact@v3 with: path: api-reference From 05f46c0b00ef6697fcb79691ed7766e22ddc8637 Mon Sep 17 00:00:00 2001 From: Dogan AY Date: Thu, 30 Jan 2025 17:42:11 +0100 Subject: [PATCH 6/9] feat: testing --- .../test/events-subscription/index.test.ts | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/packages/forestadmin-client/test/events-subscription/index.test.ts b/packages/forestadmin-client/test/events-subscription/index.test.ts index 9ae9586682..b7e88da076 100644 --- a/packages/forestadmin-client/test/events-subscription/index.test.ts +++ b/packages/forestadmin-client/test/events-subscription/index.test.ts @@ -107,7 +107,59 @@ describe('EventsSubscriptionService', () => { }); }); + describe('detectBuffering', () => { + test('should log an error after the timeout', () => { + const spy = jest.spyOn(global, 'setTimeout'); + const eventsSubscriptionService = new EventsSubscriptionService( + options, + refreshEventsHandlerService, + ); + eventsSubscriptionService.subscribeEvents(); + + expect(spy).toHaveBeenCalled(); + expect(spy).toHaveBeenCalledWith(expect.any(Function), 45000); + + const callback = spy.mock.calls[0][0]; + + callback(); + + expect(options.logger).toHaveBeenCalledWith( + 'Error', + `Unable to detect ServerSentEvents Heartbeat. + Forest Admin uses ServerSentEvents to ensure that permission cache is up to date. + It seems that your agent does not receive events from our server, this may due to buffering of events from your reverse proxy. + https://docs.forestadmin.com/developer-guide-agents-nodejs/getting-started/install/troubleshooting + `, + ); + + jest.clearAllMocks(); + }); + }); + describe('handleSeverEvents', () => { + describe('on Heartbeat', () => { + test('should clear heartbeat timeout', () => { + const eventsSubscriptionService = new EventsSubscriptionService( + options, + refreshEventsHandlerService, + ); + eventsSubscriptionService.subscribeEvents(); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + // eslint-disable-next-line no-underscore-dangle + expect(eventsSubscriptionService.heartBeatTimeout._destroyed).toBeFalsy(); + + // eslint-disable-next-line @typescript-eslint/dot-notation + events['heartbeat']({}); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + // eslint-disable-next-line no-underscore-dangle + expect(eventsSubscriptionService.heartBeatTimeout._destroyed).toBeTruthy(); + }); + }); + describe('on RefreshUsers event', () => { test('should delegate to refreshEventsHandlerService', () => { const eventsSubscriptionService = new EventsSubscriptionService( From 895073b1af0f43f96c30cca5414c63f08ed37b88 Mon Sep 17 00:00:00 2001 From: Dogan AY Date: Fri, 31 Jan 2025 10:40:13 +0100 Subject: [PATCH 7/9] fix: code review --- .../forestadmin-client/test/events-subscription/index.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/forestadmin-client/test/events-subscription/index.test.ts b/packages/forestadmin-client/test/events-subscription/index.test.ts index b7e88da076..2267d2bee7 100644 --- a/packages/forestadmin-client/test/events-subscription/index.test.ts +++ b/packages/forestadmin-client/test/events-subscription/index.test.ts @@ -127,8 +127,8 @@ describe('EventsSubscriptionService', () => { 'Error', `Unable to detect ServerSentEvents Heartbeat. Forest Admin uses ServerSentEvents to ensure that permission cache is up to date. - It seems that your agent does not receive events from our server, this may due to buffering of events from your reverse proxy. - https://docs.forestadmin.com/developer-guide-agents-nodejs/getting-started/install/troubleshooting + It seems that your agent does not receive events from our server, this may due to buffering of events from your networking infrastructure (reverse proxy). + https://docs.forestadmin.com/developer-guide-agents-nodejs/getting-started/install/troubleshooting#invalid-permissions `, ); From d101e3eda586141be75899245698ef609d17f8ed Mon Sep 17 00:00:00 2001 From: Dogan AY Date: Fri, 31 Jan 2025 10:41:16 +0100 Subject: [PATCH 8/9] fix: again --- packages/forestadmin-client/src/events-subscription/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/forestadmin-client/src/events-subscription/index.ts b/packages/forestadmin-client/src/events-subscription/index.ts index 910fea0f9b..5427e2ec40 100644 --- a/packages/forestadmin-client/src/events-subscription/index.ts +++ b/packages/forestadmin-client/src/events-subscription/index.ts @@ -25,8 +25,8 @@ export default class EventsSubscriptionService implements BaseEventsSubscription 'Error', `Unable to detect ServerSentEvents Heartbeat. Forest Admin uses ServerSentEvents to ensure that permission cache is up to date. - It seems that your agent does not receive events from our server, this may due to buffering of events from your reverse proxy. - https://docs.forestadmin.com/developer-guide-agents-nodejs/getting-started/install/troubleshooting + It seems that your agent does not receive events from our server, this may due to buffering of events from your networking infrastructure (reverse proxy). + https://docs.forestadmin.com/developer-guide-agents-nodejs/getting-started/install/troubleshooting#invalid-permissions `, ); }, 45000); From 5df2e60f69dd579069cdd24aea0a52fdf4bd0dcd Mon Sep 17 00:00:00 2001 From: Dogan AY Date: Fri, 31 Jan 2025 10:52:47 +0100 Subject: [PATCH 9/9] fix: remove whitespace --- packages/forestadmin-client/src/events-subscription/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/forestadmin-client/src/events-subscription/index.ts b/packages/forestadmin-client/src/events-subscription/index.ts index 5427e2ec40..3fe6b83209 100644 --- a/packages/forestadmin-client/src/events-subscription/index.ts +++ b/packages/forestadmin-client/src/events-subscription/index.ts @@ -25,7 +25,7 @@ export default class EventsSubscriptionService implements BaseEventsSubscription 'Error', `Unable to detect ServerSentEvents Heartbeat. Forest Admin uses ServerSentEvents to ensure that permission cache is up to date. - It seems that your agent does not receive events from our server, this may due to buffering of events from your networking infrastructure (reverse proxy). + It seems that your agent does not receive events from our server, this may due to buffering of events from your networking infrastructure (reverse proxy). https://docs.forestadmin.com/developer-guide-agents-nodejs/getting-started/install/troubleshooting#invalid-permissions `, );