Add regression tests for AppSec response body preservation on hook failure by dougqh · Pull Request #12388 · DataDog/dd-trace-java

dougqh · 2026-09-03T00:16:30Z

What Does This Do

Adds regression tests (responseHookFailureAfterBodyCapturePreservesCapturedBody) to AppSecInterceptorTest in both okhttp-2.2 and okhttp-3.0, verifying that the response body survives a throwing response hook.

Motivation

This PR was originally stacked on #12242 to fix a bug where onResponse()'s already-rebuilt response body was discarded if the response hook (publish()) threw. That fix has since landed on master as part of #12242 itself, so rebasing this branch onto master collapsed the production-code diff to nothing — the only remaining content is the two regression tests, which master didn't yet have.

Additional Notes

Applied identically to both okhttp-2.2 and okhttp-3.0 instrumentation modules.
Existing AppSecInterceptorTest suites pass for both modules; ./gradlew spotlessApply run.
/techdebt and /perf-review run over the branch diff: no findings.

Contributor Checklist

Format the title according to the contribution guidelines
Assign the type: and (comp: or inst:) labels in addition to any other useful labels
Avoid using close, fix, or any linking keywords when referencing an issue
Use solves instead, and assign the PR milestone to the issue
Update the CODEOWNERS file on source file addition, migration, or deletion
Update public documentation with any new configuration flags or behaviors
Once approved, use merge queue to merge the PR

Jira ticket: [PROJ-IDENT]

🤖 Generated with Claude Code

chain.proceed(request) was wrapped in the same try/catch that guards the AppSec request/response hooks, so any IOException from the real network call (e.g. ConnectException) was swallowed and the request was silently retried via chain.proceed(chain.request()). This double- executes non-idempotent requests on transient network failures and surfaces the retry's own failure as an unhandled error blamed on the interceptor. Narrow the try/catch to only cover the AppSec hooks so genuine I/O failures propagate normally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Covers both okhttp-2.2 and okhttp-3.0 AppSecInterceptor.intercept(): asserts an IOException from chain.proceed() propagates without being swallowed/retried, using Mockito + AgentTracer.forceRegister instead of Groovy/Spock. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…quest

span.getTag(Tags.HTTP_URL) can return null, and .toString() on it throws an NPE that silently skips the AppSec request hook for that call. Null-check instead of relying on the catch-all to swallow it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

If publish() throws inside onResponse() after the response body has already been read and rebuilt into `result`, the exception propagated out of onResponse() and caused intercept() to fall back to the original response, whose body had already been consumed. Callers then saw an empty/closed body instead of the real one. Catch and log non-blocking failures from publish() locally so the rebuilt response is always returned. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

datadog-prod-us1-5

More details

The new catch keeps the rebuilt response when the AppSec hook fails. It does not catch BlockingException.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

_{🤖 Datadog Autotest · Commit 04ae7fb · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest}

dd-octo-sts · 2026-09-03T00:32:58Z

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite	Status
Startup	🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results

Scenario	Candidate	master	Δ (95% CI of mean)
startup:insecure-bank:iast:Agent	14.04 s	14.03 s	[-0.5%; +0.6%] (no difference)
startup:insecure-bank:tracing:Agent	12.95 s	13.05 s	[-1.9%; +0.4%] (no difference)
startup:petclinic:appsec:Agent	17.07 s	17.00 s	[-0.5%; +1.3%] (no difference)
startup:petclinic:iast:Agent	17.00 s	17.04 s	[-1.2%; +0.6%] (no difference)
startup:petclinic:profiling:Agent	16.56 s	16.87 s	[-2.9%; -0.7%] (maybe better)
startup:petclinic:sca:Agent	16.94 s	16.77 s	[+0.2%; +1.8%] (maybe worse)
startup:petclinic:tracing:Agent	16.15 s	16.32 s	[-1.9%; -0.3%] (maybe better)

Commit: 78bfffca · CI Pipeline · Benchmarking Platform UI

Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

…quest

…ougqh/fix-appsec-interceptor-response-body-on-hook-failure

…nterceptor-response-body-on-hook-failure

…sponse-body-on-hook-failure' into dougqh/fix-appsec-interceptor-response-body-on-hook-failure

datadog-prod-us1-5

A wrapped exception with a null stack trace still causes a null-pointer exception after the new code installs outer probes. The same skip also gives those probes the wrong exception-chain index.

_{Open Bits AI session}

_{🤖 Datadog Autotest · Commit 47b65d5 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest}

datadog-prod-us1-5 · 2026-09-10T17:59:09Z

      while ((throwable = chainedExceptions.pollFirst()) != null) {
+        StackTraceElement[] stackTrace = throwable.getStackTrace();
+        if (stackTrace == null || stackTrace.length == 0) {
+          continue;


Guard null traces in installed exception probes

Exception Replay still fails for the FastThrow input that the new guard must support.

Assertion details

Input: An exception with a normal outer stack trace wraps an inner exception whose getStackTrace() method returns null.

Expected: Exception Replay must skip this exception safely or evaluate the installed probes without reading a null stack trace.

Actual: The new skip installs probes on the outer exception. ExceptionProbe.evaluate() later reads the null inner stack trace and throws a null-pointer exception.

Was this helpful? React 👍 or 👎
_{🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest} _{· Open Bits AI session}

datadog-prod-us1-5 · 2026-09-10T17:59:09Z

      while ((throwable = chainedExceptions.pollFirst()) != null) {
+        StackTraceElement[] stackTrace = throwable.getStackTrace();
+        if (stackTrace == null || stackTrace.length == 0) {
+          continue;


Keep the exception-chain index after a skipped trace

Snapshot processing can select the wrong throwable and assign incorrect frame tags after probe capture.

Assertion details

Input: An inner chained exception has a null or empty stack trace, and an outer exception has stack frames that can receive probes.

Expected: Each probe must store the index of the throwable that supplies its stack frames.

Actual: The continue statement does not increase chainedExceptionIdx. The code gives the outer exception the skipped inner exception's index.

Was this helpful? React 👍 or 👎
_{🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest} _{· Open Bits AI session}

onResponse() reads and closes the original response body, then rebuilds a Response with a fresh body before calling publish(). If publish()'s WAF/ gateway callback throws a non-blocking exception, that exception used to propagate out of onResponse() entirely, and intercept()'s outer catch fell back to the original (already-drained/closed) response instead of the rebuilt one -- handing the caller an empty or closed body. Guard publish() locally so a non-blocking failure there no longer discards the rebuilt response; BlockingException still propagates as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…plicate-request' into dougqh/fix-appsec-interceptor-duplicate-request

…plicate-request' into dougqh/fix-appsec-interceptor-response-body-on-hook-failure # Conflicts: # dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java # dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java

amarziali · 2026-09-11T08:00:47Z


 _Notes:_ The override label skips the workflow entirely.

+### team-freeze-guard [🔗](team-freeze-guard.yaml)


why this has been changed?

datadog-prod-us1-5

More details

Both OkHttp variants keep the rebuilt response after a non-blocking response-hook exception. They still pass a blocking exception to the caller.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

_{🤖 Datadog Autotest · Commit 7188831 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest}

…nterceptor-response-body-on-hook-failure # Conflicts: # dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java # dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/test/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptorTest.java # dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java # dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/test/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptorTest.java

datadog-prod-us1-5

More details

The added OkHttp 2 and 3 coverage exercises a response-hook exception after body capture and confirms the rebuilt response body remains available.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

_{🤖 Bits Code Review · Commit 78bfffc · @DataDog review to ask questions}

dougqh and others added 7 commits August 19, 2026 14:50

Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-re…

acf135f

…quest

Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-re…

bfe37e7

…quest

Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-re…

d8304f7

…quest

dougqh added type: bug fix Bug fix tag: ai generated Largely based on code generated by an AI or LLM inst: okhttp Square OkHttp instrumentation labels Sep 3, 2026

dougqh marked this pull request as ready for review September 3, 2026 00:24

dougqh requested a review from a team as a code owner September 3, 2026 00:24

dougqh requested review from vandonr and removed request for a team September 3, 2026 00:24

This comment has been minimized.

Sign in to view

datadog-prod-us1-5 Bot reviewed Sep 3, 2026

View reviewed changes

dougqh added 6 commits September 4, 2026 11:04

Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-re…

b09c0d4

…quest

Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-re…

8f3eb74

…quest

Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-re…

c6a82cc

…quest

Merge branch 'dougqh/fix-appsec-interceptor-duplicate-request' into d…

4a66271

…ougqh/fix-appsec-interceptor-response-body-on-hook-failure

Merge remote-tracking branch 'origin/master' into dougqh/fix-appsec-i…

bdd6ed3

…nterceptor-response-body-on-hook-failure

Merge remote-tracking branch 'origin/dougqh/fix-appsec-interceptor-re…

47b65d5

…sponse-body-on-hook-failure' into dougqh/fix-appsec-interceptor-response-body-on-hook-failure

dougqh requested review from a team as code owners September 10, 2026 17:41

dougqh removed the request for review from a team September 10, 2026 17:41

dougqh requested review from P403n1x87, amarziali and randomanderson and removed request for a team September 10, 2026 17:41

datadog-prod-us1-5 Bot reviewed Sep 10, 2026

View reviewed changes

dougqh and others added 3 commits September 10, 2026 21:27

Merge remote-tracking branch 'origin/dougqh/fix-appsec-interceptor-du…

37614a5

…plicate-request' into dougqh/fix-appsec-interceptor-duplicate-request

amarziali reviewed Sep 11, 2026

View reviewed changes

Base automatically changed from dougqh/fix-appsec-interceptor-duplicate-request to master September 11, 2026 12:57

datadog-prod-us1-5 Bot reviewed Sep 11, 2026

View reviewed changes

dougqh changed the title ~~Preserve captured response body when AppSec response hook fails~~ Preserve captured response body when AppSec response hook fails (quick fix) Sep 15, 2026

dougqh requested a review from a team as a code owner September 29, 2026 13:58

dougqh requested review from dromanol and manuel-alvarez-alvarez September 29, 2026 13:58

datadog-prod-us1-5 Bot reviewed Sep 29, 2026

View reviewed changes

dougqh changed the title ~~Preserve captured response body when AppSec response hook fails (quick fix)~~ Add regression tests for AppSec response body preservation on hook failure Sep 29, 2026

Navigation Menu

Add regression tests for AppSec response body preservation on hook failure - #12388

Add regression tests for AppSec response body preservation on hook failure#12388
dougqh wants to merge 17 commits into
masterfrom
dougqh/fix-appsec-interceptor-response-body-on-hook-failure

dougqh commented Sep 3, 2026 •

edited

Loading

Uh oh!

This comment has been minimized.

datadog-prod-us1-5 Bot left a comment

Uh oh!

dd-octo-sts Bot commented Sep 3, 2026 •

edited

Loading

Uh oh!

datadog-prod-us1-5 Bot left a comment

Uh oh!

datadog-prod-us1-5 Bot Sep 10, 2026

Uh oh!

datadog-prod-us1-5 Bot Sep 10, 2026

Uh oh!

amarziali Sep 11, 2026

Uh oh!

datadog-prod-us1-5 Bot left a comment

Uh oh!

datadog-prod-us1-5 Bot left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		_Notes:_ The override label skips the workflow entirely.

		### team-freeze-guard [🔗](team-freeze-guard.yaml)

Conversation

dougqh commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Does This Do

Motivation

Additional Notes

Contributor Checklist

Uh oh!

This comment has been minimized.

datadog-prod-us1-5 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

dd-octo-sts Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟢 Java Benchmark SLOs — All performance SLOs passed

Uh oh!

datadog-prod-us1-5 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

datadog-prod-us1-5 Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

datadog-prod-us1-5 Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

amarziali Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

datadog-prod-us1-5 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

datadog-prod-us1-5 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dougqh commented Sep 3, 2026 •

edited

Loading

dd-octo-sts Bot commented Sep 3, 2026 •

edited

Loading