✅ Test probe error handling when addProbe fails (#4754)

Author: watson

packages/browser-debugger/src/domain/deliveryApi.spec.ts

@@ -256,6 +256,106 @@ describe('deliveryApi', () => {
       expect(errorSpy).toHaveBeenCalledWith('Failed to add probe invalid-capture-expression:', jasmine.any(Error))
     })
 
+    it('should keep adding the other probes when one probe in the same response fails to initialize', async () => {
+      respondWith({
+        nextCursor: 'cursor-1',
+        updates: [
+          createProbe({ id: 'valid-before', where: { typeName: 'test.js', methodName: 'before' } }),
+          createProbe({
+            id: 'invalid',
+            where: { typeName: 'test.js', methodName: 'invalid' },
+            captureSnapshot: false,
+            captureExpressions: [
+              {
+                name: 'invalid expr',
+                expr: { dsl: 'not a valid identifier!', json: { ref: 'not a valid identifier!' } },
+              },
+            ],
+          }),
+          createProbe({ id: 'valid-after', where: { typeName: 'test.js', methodName: 'after' } }),
+        ],
+        deletions: [],
+      })
+
+      startDeliveryApiPolling(makeConfig())
+      await flushPromises()
+
+      // The failing probe must not prevent the surrounding probes from being registered.
+      expect(getProbes('test.js;before')).toBeDefined()
+      expect(getProbes('test.js;after')).toBeDefined()
+      expect(getProbes('test.js;invalid')).toBeUndefined()
+      expect(errorSpy).toHaveBeenCalledOnceWith('Failed to add probe invalid:', jasmine.any(Error))
+    })
+
+    it('should not track a probe that failed to initialize, so a later deletion is a no-op', async () => {
+      respondWith({
+        nextCursor: 'cursor-1',
+        updates: [
+          createProbe({
+            id: 'invalid',
+            captureSnapshot: false,
+            captureExpressions: [
+              {
+                name: 'invalid expr',
+                expr: { dsl: 'not a valid identifier!', json: { ref: 'not a valid identifier!' } },
+              },
+            ],
+          }),
+        ],
+        deletions: [],
+      })
+
+      startDeliveryApiPolling(makeConfig())
+      await flushPromises()
+      expect(errorSpy).toHaveBeenCalledOnceWith('Failed to add probe invalid:', jasmine.any(Error))
+
+      errorSpy.calls.reset()
+
+      // The failed probe was never tracked, so deleting it does nothing and must
+      // not attempt a removal (which would log a "Failed to remove probe" error).
+      respondWith({
+        nextCursor: 'cursor-2',
+        updates: [],
+        deletions: ['invalid'],
+      })
+      clock.tick(5000)
+      await flushPromises()
+
+      expect(errorSpy).not.toHaveBeenCalled()
+    })
+
+    it('should keep polling after a probe fails to initialize', async () => {
+      respondWith({
+        nextCursor: 'cursor-1',
+        updates: [
+          createProbe({
+            id: 'invalid',
+            captureSnapshot: false,
+            captureExpressions: [
+              {
+                name: 'invalid expr',
+                expr: { dsl: 'not a valid identifier!', json: { ref: 'not a valid identifier!' } },
+              },
+            ],
+          }),
+        ],
+        deletions: [],
+      })
+
+      startDeliveryApiPolling(makeConfig({ pollInterval: 5000 }))
+      await flushPromises()
+      expect(errorSpy).toHaveBeenCalledOnceWith('Failed to add probe invalid:', jasmine.any(Error))
+
+      // A per-probe compilation error is a data error, not a transport failure:
+      // it must not trip the circuit breaker or stop polling.
+      const callsBefore = fetchSpy.calls.count()
+      clock.tick(5000)
+      await flushPromises()
+
+      expect(fetchSpy.calls.count()).toBe(callsBefore + 1)
+      expect(warnSpy).not.toHaveBeenCalledWith(jasmine.stringMatching(/circuit breaker/i))
+    })
+
     it('should ignore log probes without a where clause', async () => {
       respondWith({
         nextCursor: 'cursor-1',

packages/browser-debugger/src/domain/probes.spec.ts

@@ -53,6 +53,18 @@ describe('probes', () => {
       const retrieved = getProbes('non-existent')
       expect(retrieved).toBeUndefined()
     })
+
+    it('should not register a probe when initialization throws', () => {
+      const probe = createProbe({
+        when: {
+          dsl: 'invalid',
+          json: { invalidOp: 'bad' } as any,
+        },
+      })
+
+      expect(() => addProbe(probe)).toThrow()
+      expect(getProbes(DEFAULT_PROBE_FUNCTION_ID)).toBeUndefined()
+    })
   })
 
   describe('removeProbe', () => {
@@ -156,25 +168,24 @@ describe('probes', () => {
       )
     })
 
-    it('should not add probe when condition compilation fails', () => {
+    it('should throw when condition compilation fails', () => {
+      const probe = createProbe({
+        when: {
+          dsl: 'invalid',
+          json: { invalidOp: 'bad' } as any,
+        },
+      })
+
       let error: unknown
       try {
-        addProbe(
-          createProbe({
-            when: {
-              dsl: 'invalid',
-              json: { invalidOp: 'bad' } as any,
-            },
-          })
-        )
+        initializeProbe(probe)
       } catch (err) {
         error = err
       }
 
       expect(error).toEqual(jasmine.any(Error))
       expect((error as Error).message).toContain('Cannot compile condition')
       expect((error as ErrorWithCause).cause).toEqual(jasmine.any(TypeError))
-      expect(getProbes(DEFAULT_PROBE_FUNCTION_ID)).toBeUndefined()
     })
 
     it('should calculate msBetweenSampling for snapshot probes', () => {