[kannanjgithub]

When RawMessageClientInterceptor or ExternalProcessorClientInterceptor encounters an internal error (such as an IOException while reading the request InputStream in sendMessage) and cancels the underlying call, the application is not synchronously aware that the call was cancelled. Callers such as ClientCalls.blockingUnaryCall immediately invoke halfClose() after sendMessage(), which previously delegated to the already-cancelled downstream call and threw
IllegalStateException("call was cancelled"), masking the underlying CANCELLED status and cause.

Fix this by tracking internal cancellation (aborted) separately from explicit application-initiated cancellation (cancelCalled):

1. RawMessageClientInterceptor (PartialForwardingClientCall):

   • Move sendSerializedMessage into PartialForwardingClientCall so sendMessage, halfClose, and cancel share per-call state.
   • Set aborted = true when sendSerializedMessage catches an IOException or RuntimeException and cancels rawCall, making subsequent sendMessage() and halfClose() calls no-ops (matching ClientCallImpl.sendMessageInternal when it cancels the underlying stream internally).
   • Set cancelCalled = true (and aborted = false) in cancel() and delegate rawCall.sendMessage(null) before stream serialization when cancelCalled is true, ensuring sendMessage() and halfClose() after explicit application cancellation still throw IllegalStateException("call was cancelled").

2. ExternalProcessorClientInterceptor (DataPlaneClientCall):

   • Set aborted = true in cancelDownstream() when triggered internally (!cancelCalled.get()), turning subsequent sendMessage(), halfClose(), and proceedWithHalfClose() calls into no-ops.
   • Set cancelCalled = true (and aborted = false) in cancel() and check !cancelCalled.get() in sendMessage() and halfClose() so explicit application cancellation enforces IllegalStateException even when requests or half-close would otherwise be buffered or sent to the ext_proc sidecar without synchronously reaching delayedCall.

Fixes the issue mentioned in an earlier comment in a PR.

[sauravzg]

Is there a potential race between cancelDownstream and cancel for the value of aborted? cancelDownstream could set aborted to true after it's ben set to false by cancel.

[kannanjgithub]

I have changed it now so that:

1. cancel() only sets cancelCalled.set(true) and calls delayedCall.cancel(message, cause) directly, while cancelDownstream() only sets aborted.set(true) and calls delayedCall.cancel(message, cause) (so neither flag ever overwrites the other).
2. sendMessage() and halfClose() check checkState(!cancelCalled.get(), "call was cancelled") before if (aborted.get()) { return; }, so if both cancelDownstream() and cancel() occur, explicit application cancellation always takes precedence without needing cancel() to reset aborted.

[sauravzg]

This doesn't hold true.
We have cases where our methods get invoked after cancellation. In particular this is again a race between cancel from application and execution of buffered halfclose from the sidecar.

While this instance of race can probably be solved moving the cancel updation in streamlock and adding checks at the callsite, we might want to audit our cancellation behavior for other such cases.

[kannanjgithub]

We should have a separate internalHalfClose method separate from the public halfClose. Only the latter should throw if application had cancelled the call already.

[sauravzg]

Shouldn't both checks include cancellation as well as abort?

Additionally, should most top level public calls be gated via cancellation and abort as well?
maybe we should start by defining our cancellation semantic i.e. "after cancellation and its atomic cleanup"

1. extproc impl absolutely cannot call any method
2. extproc impl may call other methods but all of them should behave like no-op.

[kannanjgithub]

Added check for cancellation as well.
It should just be a no-op and short-circuit ext-proc calls super.sendMessage or any public method if either aborted || cancelCalled is true.

[sauravzg]

what's this about? What does sendMessage(null) do? Is this publicly documented API or is it something specific to ext proc interceptor?

I am worried about deeply coupling this to the ext proc implementation details and also running the risk of some middle chain interceptor before proc not handling null very well.
This will tie in to my comment about defining the semantics we want after cancellation first. In this case, my thoughts would be to do an explicit failure or exception if this was called after cancellation (i.e. handle at the first point of violation) instead of delegating it.

[kannanjgithub]

Calling sendMessage(null) was simply a way to cause IllegalStateException("call was cancelled") to be thrown if the application had already cancelled.
However, doing checkState(!cancelCalled, "call was cancelled"); directly (just like in DataPlaneClientCall.sendMessage) is much cleaner.