fix: Node.js SSE reconnecting after error on 304 CDN response (#875)

DevCycleHQ · Jun 17, 2024 · ff2296a · ff2296a
1 parent cc75252
commit ff2296a
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 3 deletions.
diff --git a/lib/shared/config-manager/__tests__/environmentConfigManager.spec.ts b/lib/shared/config-manager/__tests__/environmentConfigManager.spec.ts
@@ -505,6 +505,39 @@ describe('EnvironmentConfigManager Unit Tests', () => {
             jest.useFakeTimers()
         })
 
+        it('should handle SSE connection failures and reconnect from 304 config response', async () => {
+            const envConfig = await connectToSSE()
+            mockEventSourceMethods.onopen()
+            expect(getEnvironmentConfig_mock).toBeCalledTimes(1)
+            expect(envConfig.configEtag).toEqual('etag-1')
+
+            jest.advanceTimersByTime(10000)
+            getEnvironmentConfig_mock.mockImplementation(async () =>
+                mockFetchResponse({
+                    status: 304,
+                    data: { config: {} },
+                    headers: {
+                        etag: 'etag-1',
+                        'last-modified': lastModifiedDate.toISOString(),
+                    },
+                }),
+            )
+            mockEventSourceMethods.onerror({ status: 500 })
+            expect(mockEventSourceMethods.close).toBeCalledTimes(1)
+
+            jest.advanceTimersByTime(1000)
+            // Have to use real timers here to allow the event loop to
+            // process the fetch call that is happening from the setTimeout
+            jest.useRealTimers()
+            await new Promise((resolve) => setTimeout(resolve, 10))
+
+            expect(getEnvironmentConfig_mock).toBeCalledTimes(2)
+            expect(MockEventSource).toBeCalledTimes(2)
+            expect(envConfig.configEtag).toEqual('etag-1')
+
+            jest.useFakeTimers()
+        })
+
         it('should re-connect SSE if path changes', async () => {
             const envConfig = await connectToSSE()
             mockEventSourceMethods.onopen()

diff --git a/lib/shared/config-manager/src/index.ts b/lib/shared/config-manager/src/index.ts
@@ -273,6 +273,7 @@ export class EnvironmentConfigManager {
                 `Config not modified, using cache, etag: ${this.configEtag}` +
                     `, last-modified: ${this.configLastModified}`,
             )
+            this.handleSSEConfig()
             return
         } else if (res?.status === 200 && projectConfig) {
             const lastModifiedHeader = res?.headers.get('last-modified')
@@ -329,11 +330,15 @@ export class EnvironmentConfigManager {
         )
     }
 
-    private handleSSEConfig(projectConfig: string) {
+    private handleSSEConfig(projectConfig?: string) {
         if (this.enableRealtimeUpdates) {
-            const configBody = JSON.parse(projectConfig) as ConfigBody<string>
             const originalConfigSSE = this.configSSE
-            this.configSSE = configBody.sse
+            if (projectConfig) {
+                const configBody = JSON.parse(
+                    projectConfig,
+                ) as ConfigBody<string>
+                this.configSSE = configBody.sse
+            }
 
             // Reconnect SSE if not first config fetch, and the SSE config has changed
             if (