Skip to content

Commit

Permalink
fix(client): close event source gracefully on process exits
Browse files Browse the repository at this point in the history
  • Loading branch information
Thenkei committed Jul 6, 2023
1 parent 203465b commit 0650c71
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 9 deletions.
16 changes: 12 additions & 4 deletions packages/forestadmin-client/src/events-subscription/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,22 @@ export default class EventsSubscriptionService implements BaseEventsSubscription
}

private onEventError(event: { type: string; status?: number; message?: string }) {
if (event.status === 502) {
this.options.logger('Debug', 'Server Event - Connection lost');
const { status, message } = event;

if ([502, 503, 504].includes(status)) {
this.options.logger('Debug', 'Server Event - Connection lost trying to reconnect…');

return;
}

if (event.message)
this.options.logger('Warn', `Server Event - Error: ${JSON.stringify(event)}`);
if (status === 404)
throw new Error(
'Forest Admin server failed to find the environment ' +
'related to the envSecret you configured. ' +
'Can you check that you copied it properly during initialization?',
);

if (message) this.options.logger('Warn', `Server Event - Error: ${JSON.stringify(event)}`);
}

private onEventOpenAgain() {
Expand Down
31 changes: 26 additions & 5 deletions packages/forestadmin-client/test/events-subscription/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ describe('EventsSubscriptionService', () => {

describe('onEventError', () => {
describe('when error status is Bad Gateway (502)', () => {
test('should delegate to refreshEventsHandlerService', async () => {
test('should warn the user', async () => {
const eventsSubscriptionService = new EventsSubscriptionService(
options,
refreshEventsHandlerService,
Expand All @@ -204,12 +204,33 @@ describe('EventsSubscriptionService', () => {
// eslint-disable-next-line @typescript-eslint/dot-notation
events['error']({ status: 502 });

expect(options.logger).toHaveBeenCalledWith('Debug', 'Server Event - Connection lost');
expect(options.logger).toHaveBeenCalledWith(
'Debug',
'Server Event - Connection lost trying to reconnect…',
);
});
});

describe('when error status is Not Found (404)', () => {
test('should warn the user', async () => {
const eventsSubscriptionService = new EventsSubscriptionService(
options,
refreshEventsHandlerService,
);

eventsSubscriptionService.subscribeEvents();

// eslint-disable-next-line @typescript-eslint/dot-notation
expect(() => events['error']({ status: 404 })).toThrow(
'Forest Admin server failed to find the environment ' +
'related to the envSecret you configured. ' +
'Can you check that you copied it properly during initialization?',
);
});
});

describe('other error case', () => {
test('should delegate to refreshEventsHandlerService', async () => {
test('should warn with error message information the user', async () => {
const eventsSubscriptionService = new EventsSubscriptionService(
options,
refreshEventsHandlerService,
Expand All @@ -219,12 +240,12 @@ describe('EventsSubscriptionService', () => {

// eslint-disable-next-line @typescript-eslint/dot-notation
events['error']({
status: 404,
status: 403,
message: 'some error message that might help customer to track issues',
});
expect(options.logger).toHaveBeenCalledWith(
'Warn',
'Server Event - Error: {"status":404,"message":"some error message that' +
'Server Event - Error: {"status":403,"message":"some error message that' +
' might help customer to track issues"}',
);
});
Expand Down

0 comments on commit 0650c71

Please sign in to comment.