[DI] Don't fail to trigger if one of multiple probe conditions throws (#5715)

If two probes are on the same line, and both have a condition, but one
condition throws and the other evaluates to true, the one that evaluated
to true should still trigger the breakpoint and get collected.

Author: watson

integration-tests/debugger/basic.spec.js

@@ -390,6 +390,46 @@ describe('Dynamic Instrumentation', function () {
           }
         })
 
+        it('trigger on met condition, even if other condition throws (all have conditions)', function (done) {
+          let installed = 0
+          // this condition will throw because `foo` is not defined
+          const rcConfig1 = t.generateRemoteConfig({ when: { json: { eq: [{ ref: 'foo' }, 'bar'] } } })
+          const rcConfig2 = t.generateRemoteConfig({
+            when: { json: { eq: [{ getmember: [{ getmember: [{ ref: 'request' }, 'params'] }, 'name'] }, 'bar'] } }
+          })
+          const expectedPayloads = new Map([
+            [rcConfig2.config.id, {
+              ddsource: 'dd_debugger',
+              service: 'node',
+              debugger: { diagnostics: { probeId: rcConfig2.config.id, probeVersion: 0, status: 'EMITTING' } }
+            }]
+          ])
+
+          t.agent.on('debugger-diagnostics', ({ payload }) => {
+            payload.forEach((event) => {
+              const { diagnostics } = event.debugger
+              if (diagnostics.status === 'INSTALLED') {
+                if (++installed === 2) {
+                  t.axios.get(t.breakpoint.url).catch(done)
+                }
+              } else if (diagnostics.status === 'EMITTING') {
+                const expected = expectedPayloads.get(diagnostics.probeId)
+                assert.ok(expected, `expected payload not found for probe ${diagnostics.probeId}`)
+                expectedPayloads.delete(diagnostics.probeId)
+                assertObjectContains(event, expected)
+              }
+            })
+            endIfDone()
+          })
+
+          t.agent.addRemoteConfig(rcConfig1)
+          t.agent.addRemoteConfig(rcConfig2)
+
+          function endIfDone () {
+            if (expectedPayloads.size === 0) done()
+          }
+        })
+
         it('should only trigger the probes whos conditions are met (not all have conditions)', function (done) {
           let installed = 0
           const rcConfig1 = t.generateRemoteConfig({

packages/dd-trace/src/debugger/devtools_client/breakpoints.js

@@ -187,11 +187,13 @@ function stop () {
 
 // Only if all probes have a condition can we use a compound condition.
 // Otherwise, we need to evaluate each probe individually once the breakpoint is hit.
-// TODO: Handle errors - if there's 2 conditions, and one fails but the other returns true, we should still pause the
-// breakpoint
 function compileCompoundCondition (probes) {
+  if (probes.length === 1) return probes[0].condition
+
   return probes.every(p => p.condition)
-    ? probes.map(p => p.condition).filter(Boolean).join(' || ')
+    ? probes
+      .map((p) => `(() => { try { return ${p.condition} } catch { return false } })()`)
+      .join(' || ')
     : undefined
 }
 

packages/dd-trace/test/debugger/devtools_client/breakpoints.spec.js

@@ -233,7 +233,9 @@ describe('breakpoints', function () {
             lineNumber: 9,
             columnNumber: 0
           },
-          condition: '(foo) === (42) || (foo) === (43)'
+          condition:
+            '(() => { try { return (foo) === (42) } catch { return false } })() || ' +
+            '(() => { try { return (foo) === (43) } catch { return false } })()'
         })
       })