test(agent): reject when no matching trace arrives before the timeout#9177
Conversation
Overall package sizeSelf size: 6.66 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.1 | 122.62 kB | 437.94 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 1228f05 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-07-08 18:46:36 Comparing candidate commit 1228f05 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2310 metrics, 48 unstable metrics.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9177 +/- ##
==========================================
- Coverage 96.54% 96.54% -0.01%
==========================================
Files 915 915
Lines 121018 121034 +16
Branches 20693 20600 -93
==========================================
+ Hits 116837 116849 +12
- Misses 4181 4185 +4 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34e4dc98f0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
An `assertSomeTraces` / `expectSomeSpan` expectation whose span never arrives left the returned promise unsettled: the rejection timeout only fired when errors had accumulated, so with zero payloads the callback did nothing and the spec hung until Mocha's 5s per-test timeout killed it with a generic "Timeout of 5000ms exceeded" instead of failing at the intended 1s window with the real cause. The rejection timeout now always settles the promise: the accumulated first/aggregate error when a payload matched but assertions failed, and otherwise a fresh error naming the timeout window so a missing span points the reader at "no matching trace received within <timeoutMs>ms".
…t reject Rejecting the assertSomeTraces promise at its timeout broke tests that relied on the previous never-settling behaviour: 1. "Should not be traced" cases registered an expectation that must never match, left the promise unawaited, and passed via a separate setTimeout(done). The reject now fires into that path, calling done() twice or surfacing a stray "No matching trace". A new assertNoTraces resolves when nothing matches before the timeout and rejects only when a forbidden trace arrives; the negative sites move to it. 2. Positive tests whose span legitimately lands after 1s (restify <5 uncaught exceptions ~5s, the openai/anthropic dead-baseURL retry backoff, kafka consumer delivery, aws-sdk cold connect, the http Node-20 timeout cases) now set an explicit timeoutMs. openai/anthropic instead drop retries with maxRetries: 0 since the endpoint is dead on purpose. The LLMObs harness gets a longer window because every call is a real VCR round-trip. 3. mongodb-core's BigInt test returns done() early on the version that throws synchronously; the assertion now registers only once the command starts, so no expectation is left to reject afterwards.
34e4dc9 to
15230c5
Compare
An assertSomeTraces expectation that is armed but never consumed can keep its rejection timer running after the test ends. The timer then rejects a promise nobody awaits a whole timeout window later, surfacing as an unhandled rejection on an unrelated test. 1. Each expectation promise carries a cancel() that disarms its timer and drops the handler; the consumer that armed it cancels it in its own teardown. 2. The framework reset() disarms every still-armed expectation, then throws to fail the just-finished test by name. Bare subscribe() handlers have no timer and are never counted. 3. The invalid-type SDK test no longer races getEvents() against a timeout, which abandoned a poll loop that kept draining span requests and starving later tests.
15230c5 to
ce22372
Compare
This PR's teardown leak guard counts any handler left in the set as armed, but runCallbackAgainstTraces kept its handler on the timeout and rejectFirst settle paths, so the guard rejected expectations it had already settled. Drop the handler on every settle path. With the guard trustworthy, three specs leaked real expectations: 1. aws-sdk sqs: timeoutMs sat on `.catch()` rather than assertSomeTraces, so the suppressed-consumer expectation ran on the 1000ms default and stayed armed past the 250ms done(). 2. google-cloud-pubsub dsm and index: the DSM checkpoint expectations were never awaited. Register before publishing, then await, and read the expected pathway hash little-endian to match DataStreamsProcessor's readBigUInt64LE decode.
`should allow disabling a specific service` and its sqs consumer-span-kind sibling paired two fire-and-forget assertSomeTraces expectations on a 100ms window with a fixed setTimeout(done, 250) asserting a match counter. On a cold localstack the enabled request's span lands after the 100ms window, so its expectation times out, the counter stays 0, and the guard asserts 0 !== 1. Assert the disabled request's absence with assertNoTraces and await the enabled request's assertSomeTraces, which is what assertNoTraces documents in place of a bare assertSomeTraces plus a separate setTimeout(done, …).
…e timeout This PR makes assertSomeTraces reject at its timeout instead of waiting out the mocha deadline. The full-agentic-call test registers the expectation, then spawns the claude subprocess and runs several steps before the trace flushes, which takes longer than the 1000ms default, so the expectation rejected before the trace ever arrived. Opt into a 10s timeoutMs and raise the block's mocha timeout to 15s.
…#9177) * test(agent): reject when no matching trace arrives before the timeout An `assertSomeTraces` / `expectSomeSpan` expectation whose span never arrives left the returned promise unsettled: the rejection timeout only fired when errors had accumulated, so with zero payloads the callback did nothing and the spec hung until Mocha's 5s per-test timeout killed it with a generic "Timeout of 5000ms exceeded" instead of failing at the intended 1s window with the real cause. The rejection timeout now always settles the promise: the accumulated first/aggregate error when a payload matched but assertions failed, and otherwise a fresh error naming the timeout window so a missing span points the reader at "no matching trace received within <timeoutMs>ms". * test(agent): add assertNoTraces and give slow spans room after timeout reject Rejecting the assertSomeTraces promise at its timeout broke tests that relied on the previous never-settling behaviour: 1. "Should not be traced" cases registered an expectation that must never match, left the promise unawaited, and passed via a separate setTimeout(done). The reject now fires into that path, calling done() twice or surfacing a stray "No matching trace". A new assertNoTraces resolves when nothing matches before the timeout and rejects only when a forbidden trace arrives; the negative sites move to it. 2. Positive tests whose span legitimately lands after 1s (restify <5 uncaught exceptions ~5s, the openai/anthropic dead-baseURL retry backoff, kafka consumer delivery, aws-sdk cold connect, the http Node-20 timeout cases) now set an explicit timeoutMs. openai/anthropic instead drop retries with maxRetries: 0 since the endpoint is dead on purpose. The LLMObs harness gets a longer window because every call is a real VCR round-trip. 3. mongodb-core's BigInt test returns done() early on the version that throws synchronously; the assertion now registers only once the command starts, so no expectation is left to reject afterwards. * test(agent): fail loudly on leaked trace expectations An assertSomeTraces expectation that is armed but never consumed can keep its rejection timer running after the test ends. The timer then rejects a promise nobody awaits a whole timeout window later, surfacing as an unhandled rejection on an unrelated test. 1. Each expectation promise carries a cancel() that disarms its timer and drops the handler; the consumer that armed it cancels it in its own teardown. 2. The framework reset() disarms every still-armed expectation, then throws to fail the just-finished test by name. Bare subscribe() handlers have no timer and are never counted. 3. The invalid-type SDK test no longer races getEvents() against a timeout, which abandoned a poll loop that kept draining span requests and starving later tests. * test(agent): drop settled handlers and fix the leaks the guard exposed This PR's teardown leak guard counts any handler left in the set as armed, but runCallbackAgainstTraces kept its handler on the timeout and rejectFirst settle paths, so the guard rejected expectations it had already settled. Drop the handler on every settle path. With the guard trustworthy, three specs leaked real expectations: 1. aws-sdk sqs: timeoutMs sat on `.catch()` rather than assertSomeTraces, so the suppressed-consumer expectation ran on the 1000ms default and stayed armed past the 250ms done(). 2. google-cloud-pubsub dsm and index: the DSM checkpoint expectations were never awaited. Register before publishing, then await, and read the expected pathway hash little-endian to match DataStreamsProcessor's readBigUInt64LE decode. * test(aws-sdk): make the disabling tests deterministic `should allow disabling a specific service` and its sqs consumer-span-kind sibling paired two fire-and-forget assertSomeTraces expectations on a 100ms window with a fixed setTimeout(done, 250) asserting a match counter. On a cold localstack the enabled request's span lands after the 100ms window, so its expectation times out, the counter stays 0, and the guard asserts 0 !== 1. Assert the disabled request's absence with assertNoTraces and await the enabled request's assertSomeTraces, which is what assertNoTraces documents in place of a bare assertSomeTraces plus a separate setTimeout(done, …). * test(claude-agent-sdk): give the agentic-call assertion room under the timeout This PR makes assertSomeTraces reject at its timeout instead of waiting out the mocha deadline. The full-agentic-call test registers the expectation, then spawns the claude subprocess and runs several steps before the trace flushes, which takes longer than the 1000ms default, so the expectation rejected before the trace ever arrived. Opt into a 10s timeoutMs and raise the block's mocha timeout to 15s.
…#9177) * test(agent): reject when no matching trace arrives before the timeout An `assertSomeTraces` / `expectSomeSpan` expectation whose span never arrives left the returned promise unsettled: the rejection timeout only fired when errors had accumulated, so with zero payloads the callback did nothing and the spec hung until Mocha's 5s per-test timeout killed it with a generic "Timeout of 5000ms exceeded" instead of failing at the intended 1s window with the real cause. The rejection timeout now always settles the promise: the accumulated first/aggregate error when a payload matched but assertions failed, and otherwise a fresh error naming the timeout window so a missing span points the reader at "no matching trace received within <timeoutMs>ms". * test(agent): add assertNoTraces and give slow spans room after timeout reject Rejecting the assertSomeTraces promise at its timeout broke tests that relied on the previous never-settling behaviour: 1. "Should not be traced" cases registered an expectation that must never match, left the promise unawaited, and passed via a separate setTimeout(done). The reject now fires into that path, calling done() twice or surfacing a stray "No matching trace". A new assertNoTraces resolves when nothing matches before the timeout and rejects only when a forbidden trace arrives; the negative sites move to it. 2. Positive tests whose span legitimately lands after 1s (restify <5 uncaught exceptions ~5s, the openai/anthropic dead-baseURL retry backoff, kafka consumer delivery, aws-sdk cold connect, the http Node-20 timeout cases) now set an explicit timeoutMs. openai/anthropic instead drop retries with maxRetries: 0 since the endpoint is dead on purpose. The LLMObs harness gets a longer window because every call is a real VCR round-trip. 3. mongodb-core's BigInt test returns done() early on the version that throws synchronously; the assertion now registers only once the command starts, so no expectation is left to reject afterwards. * test(agent): fail loudly on leaked trace expectations An assertSomeTraces expectation that is armed but never consumed can keep its rejection timer running after the test ends. The timer then rejects a promise nobody awaits a whole timeout window later, surfacing as an unhandled rejection on an unrelated test. 1. Each expectation promise carries a cancel() that disarms its timer and drops the handler; the consumer that armed it cancels it in its own teardown. 2. The framework reset() disarms every still-armed expectation, then throws to fail the just-finished test by name. Bare subscribe() handlers have no timer and are never counted. 3. The invalid-type SDK test no longer races getEvents() against a timeout, which abandoned a poll loop that kept draining span requests and starving later tests. * test(agent): drop settled handlers and fix the leaks the guard exposed This PR's teardown leak guard counts any handler left in the set as armed, but runCallbackAgainstTraces kept its handler on the timeout and rejectFirst settle paths, so the guard rejected expectations it had already settled. Drop the handler on every settle path. With the guard trustworthy, three specs leaked real expectations: 1. aws-sdk sqs: timeoutMs sat on `.catch()` rather than assertSomeTraces, so the suppressed-consumer expectation ran on the 1000ms default and stayed armed past the 250ms done(). 2. google-cloud-pubsub dsm and index: the DSM checkpoint expectations were never awaited. Register before publishing, then await, and read the expected pathway hash little-endian to match DataStreamsProcessor's readBigUInt64LE decode. * test(aws-sdk): make the disabling tests deterministic `should allow disabling a specific service` and its sqs consumer-span-kind sibling paired two fire-and-forget assertSomeTraces expectations on a 100ms window with a fixed setTimeout(done, 250) asserting a match counter. On a cold localstack the enabled request's span lands after the 100ms window, so its expectation times out, the counter stays 0, and the guard asserts 0 !== 1. Assert the disabled request's absence with assertNoTraces and await the enabled request's assertSomeTraces, which is what assertNoTraces documents in place of a bare assertSomeTraces plus a separate setTimeout(done, …). * test(claude-agent-sdk): give the agentic-call assertion room under the timeout This PR makes assertSomeTraces reject at its timeout instead of waiting out the mocha deadline. The full-agentic-call test registers the expectation, then spawns the claude subprocess and runs several steps before the trace flushes, which takes longer than the 1000ms default, so the expectation rejected before the trace ever arrived. Opt into a 10s timeoutMs and raise the block's mocha timeout to 15s.
Summary
An
assertSomeTraces/expectSomeSpanexpectation whose span never arrives left the returned promise unsettled. The rejection timeout only fired when errors had accumulated (if (errors.length)), so with zero payloads the callback did nothing and the spec hung until Mocha's 5s per-test timeout killed it with a genericTimeout of 5000ms exceeded— instead of failing at the intended 1s window with the real cause.The rejection timeout now always settles the promise: the accumulated first/aggregate error when a payload matched but assertions failed, and otherwise a fresh error naming the timeout window so a missing span reports
No matching trace received within <timeoutMs>ms. The success andrejectFirstpaths stillclearTimeoutbefore settling, so a cleared timer cannot reject an already-resolved promise.Regression test in
agent.spec.jsloads the mock agent, callsassertSomeTraceswith a 200ms timeout and no payload, and asserts it rejects with the new message well under 1s. Onmasterthe same test hangs the full 5s and fails with the generic Mocha timeout.