Stop AppSecInterceptor from silently retrying failed okhttp requests by dougqh · Pull Request #12242 · DataDog/dd-trace-java

dougqh · 2026-08-19T18:50:53Z

What Does This Do

Narrows the try/catch in AppSecInterceptor.intercept() (both okhttp-2.2 and okhttp-3.0) so it only guards the AppSec-specific hook logic (sampleRequest/onRequest/onResponse), not the real chain.proceed() network call. Genuine I/O failures now propagate normally on the first (and only) attempt, exactly as they would without this instrumentation.

Motivation

AppSecInterceptor.intercept() previously wrapped the real chain.proceed(request) call inside the same try/catch (Exception e) that guards the AppSec hooks. Any IOException from the actual outbound request (e.g. java.net.ConnectException on a transient network failure) was caught by that handler, logged at debug, and the request was silently retried via chain.proceed(chain.request()). This means:

Non-idempotent requests (POST/PUT) can be executed twice on a transient failure.
If the retry also fails, its IOException propagates unhandled and gets misattributed to AppSecInterceptor.intercept in error tracking, when the real cause is just an ordinary network failure.

Found via Datadog Error Tracking issue 7d2a0a46-aab1-11f0-a468-da7ad0900002 (38k+ occurrences) — automated root cause analysis confirmed the stack shows ConnectException propagating through AppSecInterceptor:57/58, matching this code path exactly. Same shape exists in the okhttp2 sibling (db728b88-b44c-11f0-a740-da7ad0900002).

Additional Notes

No dedicated unit test exists for AppSecInterceptor behavior itself in either module; happy to add one covering the retry regression if desired.

Test plan

./gradlew :dd-java-agent:instrumentation:okhttp:okhttp-3.0:compileJava :dd-java-agent:instrumentation:okhttp:okhttp-2.2:compileJava — BUILD SUCCESSFUL
./gradlew :dd-java-agent:instrumentation:okhttp:okhttp-3.0:spotlessApply :dd-java-agent:instrumentation:okhttp:okhttp-2.2:spotlessApply — applied, no additional changes
./gradlew :dd-java-agent:instrumentation:okhttp:okhttp-3.0:test :dd-java-agent:instrumentation:okhttp:okhttp-2.2:test — BUILD SUCCESSFUL, existing instrumentation tests pass

🤖 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>

dd-octo-sts · 2026-08-19T19:14:32Z

🟢 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.79 s	14.69 s	[-0.1%; +1.4%] (no difference)
startup:insecure-bank:tracing:Agent	13.56 s	13.72 s	[-2.0%; -0.4%] (maybe better)
startup:petclinic:appsec:Agent	17.02 s	16.90 s	[-0.2%; +1.7%] (no difference)
startup:petclinic:iast:Agent	16.89 s	17.09 s	[-1.9%; -0.4%] (maybe better)
startup:petclinic:profiling:Agent	16.55 s	16.74 s	[-2.1%; -0.2%] (maybe better)
startup:petclinic:sca:Agent	16.98 s	16.93 s	[-0.7%; +1.4%] (no difference)
startup:petclinic:tracing:Agent	16.17 s	16.18 s	[-1.0%; +0.8%] (no difference)

Commit: 37614a5c · 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.

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

datadog-datadog-prod-us1

More details

The network call is outside both hook exception handlers. The first I/O failure now passes to the caller, with no silent retry in either OkHttp integration.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

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

sarahchen6

LGTM

…quest

datadog-datadog-prod-us1

A sampled response body can become closed or empty when the response hook fails after body capture. Both interceptor copies return the consumed original response instead of the rebuilt response.

_{Open Bits AI session}

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

amarziali

Thanks for having fixed that. I left a minor improvement

…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>

…quest

dougqh · 2026-09-10T12:46:44Z

@ygree friendly nudge — this one's been waiting on your review, would appreciate a look when you have a chance.

ygree · 2026-09-10T22:53:52Z

@codex review

chatgpt-codex-connector · 2026-09-10T22:54:13Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-10T22:58:07.984711Z	`c6a82cc`	Manual request
🔒 Security Review	✅ Completed 2026-09-10T22:59:12.792210Z	`c6a82cc`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c6a82ccee1

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-10T22:59:11Z

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: c6a82ccee1

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

ygree

IDM approved. Please also check codex's finding: https://github.com/DataDog/dd-trace-java/pull/12242/changes#r3984229637

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

dd-octo-sts · 2026-09-11T11:39:58Z

/merge

gh-worker-devflow-routing-ef8351 · 2026-09-11T11:40:02Z

View all feedbacks in Devflow UI.

2026-09-11 11:40:02 UTC ℹ️ Start processing command /merge

2026-09-11 11:40:07 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in master is approximately 1h (p90).

2026-09-11 12:57:41 UTC ℹ️ MergeQueue: This merge request was merged

…12242) Stop AppSecInterceptor from silently retrying failed okhttp requests 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> Add JUnit5 regression test for AppSecInterceptor silent-retry fix 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> Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-request Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-request Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-request Guard against NPE reading HTTP_URL tag in AppSecInterceptor 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> Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-request Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-request Merge branch 'master' into dougqh/fix-appsec-interceptor-duplicate-request Preserve rebuilt response body when the AppSec response hook fails 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> Merge remote-tracking branch 'origin/dougqh/fix-appsec-interceptor-duplicate-request' into dougqh/fix-appsec-interceptor-duplicate-request Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>

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

This comment has been minimized.

Sign in to view

dougqh marked this pull request as ready for review August 21, 2026 13:29

dougqh requested a review from a team as a code owner August 21, 2026 13:29

dougqh requested review from ygree and removed request for a team August 21, 2026 13:29

dougqh and others added 2 commits August 21, 2026 09:55

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

acf135f

…quest

datadog-datadog-prod-us1 Bot reviewed Aug 21, 2026

View reviewed changes

sarahchen6 approved these changes Sep 1, 2026

View reviewed changes

dougqh enabled auto-merge September 1, 2026 20:53

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

bfe37e7

…quest

dougqh requested a review from a team as a code owner September 2, 2026 01:17

dougqh requested review from amarziali and removed request for a team September 2, 2026 01:17

datadog-datadog-prod-us1 Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread ...okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java

amarziali approved these changes Sep 2, 2026

View reviewed changes

Comment thread ...okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java Outdated

dougqh and others added 2 commits September 2, 2026 19:57

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

d8304f7

…quest

dougqh mentioned this pull request Sep 3, 2026

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

Open

dougqh added 3 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

chatgpt-codex-connector Bot reviewed Sep 10, 2026

View reviewed changes

Comment thread ...okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java

ygree approved these changes Sep 10, 2026

View reviewed changes

dougqh and others added 2 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

dougqh added this pull request to the merge queue Sep 11, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 11, 2026

gh-worker-dd-mergequeue-cf854d Bot merged commit 40a0f9d into master Sep 11, 2026
603 checks passed

gh-worker-dd-mergequeue-cf854d Bot deleted the dougqh/fix-appsec-interceptor-duplicate-request branch September 11, 2026 12:57

github-actions Bot added this to the 1.67.0 milestone Sep 11, 2026

Conversation

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

Uh oh!

What Does This Do

Motivation

Additional Notes

Test plan

Uh oh!

This comment has been minimized.

dd-octo-sts Bot commented Aug 19, 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-datadog-prod-us1 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

sarahchen6 left a comment

Choose a reason for hiding this comment

Uh oh!

datadog-datadog-prod-us1 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amarziali left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dougqh commented Sep 10, 2026

Uh oh!

ygree commented Sep 10, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Sep 10, 2026

🛡️ Codex Security Review · Automatically triggered

Uh oh!

ygree left a comment

Choose a reason for hiding this comment

Uh oh!

dd-octo-sts Bot commented Sep 11, 2026

Uh oh!

gh-worker-devflow-routing-ef8351 Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dougqh commented Aug 19, 2026 •

edited

Loading

dd-octo-sts Bot commented Aug 19, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 10, 2026 •

edited

Loading

gh-worker-devflow-routing-ef8351 Bot commented Sep 11, 2026 •

edited

Loading