Skip to content

Commit 872a129

Browse files
committed
Fix: propagate commit exceptions instead of swallowing them in BlockFailureReporter
Remove the catch(Exception) around brf.tryCommitBlockingResponse() in tryCommitAndReport() so it matches the Netty reference implementation (PR #12316), which only branches on the boolean return value. Several call sites (CommitActionInstrumentation in 5.5/7.0, ParsePartsInstrumentation, ParsedBodyParametersInstrumentation) rely on their enclosing advice's suppress = Throwable.class to abort the whole method when the commit throws, gating success-path side effects (closing the connection, injecting a BlockingException, marking the segment effectively blocked). Swallowing the exception inside tryCommitAndReport made those side effects run unconditionally even when nothing was actually committed to the client. Removing the catch restores the pre-existing control flow; exception-based commit failures are now a known gap not reported to block_failure telemetry, same as Netty. Addresses Codex review comment on PR #12493.
1 parent 3d3b9bb commit 872a129

1 file changed

Lines changed: 12 additions & 12 deletions

File tree

  • dd-java-agent/instrumentation/tomcat/tomcat-common/src/main/java/datadog/trace/instrumentation/tomcat

‎dd-java-agent/instrumentation/tomcat/tomcat-common/src/main/java/datadog/trace/instrumentation/tomcat/BlockFailureReporter.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
import datadog.trace.api.gateway.Flow;
66
import datadog.trace.api.gateway.RequestContext;
77
import datadog.trace.api.gateway.RequestContextSlot;
8-
import org.slf4j.Logger;
9-
import org.slf4j.LoggerFactory;
108

119
/**
1210
* Catalina-independent counterpart of {@link TomcatBlockingHelper}: commits a blocking response
@@ -21,7 +19,6 @@
2119
* every version in that range.
2220
*/
2321
public class BlockFailureReporter {
24-
private static final Logger log = LoggerFactory.getLogger(BlockFailureReporter.class);
2522

2623
/**
2724
* Commits a blocking response through the {@link BlockResponseFunction} registered on the given
@@ -31,9 +28,18 @@ public class BlockFailureReporter {
3128
* <p>This is the single choke point shared by all Tomcat blocking call sites so the {@code
3229
* block_failure} telemetry is not duplicated inline.
3330
*
31+
* <p>Exceptions thrown by the commit attempt are deliberately propagated instead of being
32+
* converted into a {@code false} return: the calling advice methods declare {@code suppress =
33+
* Throwable.class} and rely on the whole advice being aborted so that their blocking success-path
34+
* side effects (closing the connection, injecting a {@link
35+
* datadog.appsec.api.blocking.BlockingException}, marking the trace segment effectively blocked)
36+
* are skipped when no response was committed. Such exception-based commit failures are therefore
37+
* not reported to the {@code block_failure} telemetry, which is the same known gap as the Netty
38+
* implementation this mirrors.
39+
*
3440
* @return {@code true} if the blocking response was committed, {@code false} otherwise (including
3541
* when no {@link BlockResponseFunction} is registered, in which case nothing was attempted
36-
* and no failure is reported, or when the commit attempt threw).
42+
* and no failure is reported).
3743
*/
3844
public static boolean tryCommitAndReport(
3945
RequestContext reqCtx, Flow.Action.RequestBlockingAction rba) {
@@ -42,14 +48,8 @@ public static boolean tryCommitAndReport(
4248
// nothing was attempted, so this is not a block failure
4349
return false;
4450
}
45-
try {
46-
if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) {
47-
return true;
48-
}
49-
} catch (Exception e) {
50-
log.debug("Error committing blocking response", e);
51-
reportBlockFailure(reqCtx);
52-
return false;
51+
if (brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba)) {
52+
return true;
5353
}
5454
reportBlockFailure(reqCtx);
5555
return false;

0 commit comments

Comments
 (0)