fix(bigtable): report the real error behind session-path UNKNOWNs

[mutianf]

Problem

DivertingUnaryCallable.translateException is the only place the session path converts a failure into the caller's exception, and it defaulted to UNKNOWN for anything that is not a StatusException/StatusRuntimeException.

A production incident surfaced application-visible UNKNOWN errors with no matching UNKNOWN in CSM and none on the server. Because the default discarded the status and said nothing about the throwable, the bare code left nothing to diagnose from — the client's last chance to report what actually failed was spent emitting three uninformative characters.

Changes

Walk the full cause chain when looking for a gRPC status. Unwrapping stopped at CompletionException/ExecutionException, so a valid StatusRuntimeException wrapped in any other type lost its code and was reported as UNKNOWN. Bounded at depth 32 with a self-cycle guard.

Map CancellationException to CANCELLED, matching csm.attributes.Util#extractStatus. The two mappings disagreed, so the same failure could be CANCELLED in metrics and UNKNOWN to the caller.

When the fall-through to UNKNOWN is genuine, name the throwable:

Session operation failed with an error that carries no gRPC status; reporting UNKNOWN.
Cause chain: java.lang.IllegalStateException. Message: Unary rpc completed OK but missing result

The first occurrence per callable is logged at WARNING with a full stack, FINE thereafter — a storm is exactly when this fires most, and an unconditional WARNING would flood the log at the moment an operator can least afford it.

Reviewer notes

This changes observable status codes, not just messages: failures that were UNKNOWN only because their status sat one wrapper too deep now report their real code, and cancellations become CANCELLED. That is the intent, but it is user-visible. Blast radius is limited — ApiExceptionFactory.createException(..., false) hard-codes retryable=false, so gax retry behavior does not key off this.

Deliberately not included: no counter was added to SessionPoolMap.apply, since every throw it catches funnels through translateException anyway and the logging covers it without threading a Metrics dependency through a new layer. UnaryResponseFuture's OK-without-message IllegalStateException is also left alone — the new message names it exactly if it ever fires, and changing a status on a defensive "can't happen" branch is a separate decision.

Tests

DivertingUnaryCallableTest (14 tests) covers the mapping: which throwables become UNKNOWN, that Status-bearing ones keep their code through arbitrary wrappers, the CancellationException case, the self-referential cause chain, and the message content — including the null-message NullPointerException, where a bare cause.getMessage() would have given an operator literally nothing.

SessionPathErrorEscapeTest (4 tests) injects faults at the seam between SessionPoolMap.apply and the session machinery to establish which throw sites can produce an application UNKNOWN at all:

Site	Behavior
metrics.newTableTracer throws	escapes TableBase.readRow synchronously; listener never notified; zero CSM records
SessionPool.newCall throws	does not escape — RetryingVRpc.start converts it to CANCELLED, recorded in CSM
vRPC closes OK with no message	caller gets a bare IllegalStateException while CSM and the server both record OK

The second row is worth calling out: it rules the SessionList close/drain race out as a source of application UNKNOWN, since it would appear as CANCELLED in both places.

Full data.v2.internal suite passes (1021 tests).

[gemini-code-assist]

Code Review

This pull request enhances exception translation in DivertingUnaryCallable by walking the cause chain of throwables to extract gRPC status codes, logging unrecognized errors with their cause chains to aid diagnostics. It also introduces comprehensive tests in SessionPathErrorEscapeTest and DivertingUnaryCallableTest to validate error propagation. The review feedback suggests moving the CancellationException check inside the cause-walking loop to correctly identify wrapped cancellations, and adding a null check in describeUnrecognized to prevent a potential NullPointerException.

[gemini-code-assist]

The current implementation of findStatusCode only checks if the top-level throwable t is an instance of CancellationException. If a CancellationException is wrapped inside another exception (such as a RuntimeException or custom wrapper), it will not be detected, and the method will return null (which ultimately maps to UNKNOWN instead of CANCELLED).

Moving the CancellationException check inside the cause-walking loop ensures that wrapped cancellations are correctly identified and mapped to CANCELLED, matching the behavior of StatusRuntimeException and StatusException.

Suggested change

	private static Status.Code findStatusCode(Throwable t) {
	if (t instanceof CancellationException) {
	return Status.Code.CANCELLED;
	}
	if (cause instanceof StatusException) {
	code = ((StatusException) cause).getStatus().getCode();
	Throwable current = t;
	for (int depth = 0; current != null && depth < MAX_CAUSE_DEPTH; depth++) {
	if (current instanceof StatusRuntimeException) {
	return ((StatusRuntimeException) current).getStatus().getCode();
	}
	if (current instanceof StatusException) {
	return ((StatusException) current).getStatus().getCode();
	}
	Throwable next = current.getCause();
	if (next == current) {
	break; // self-referential chain
	}
	current = next;
	}
	return null;
	}
	private static Status.Code findStatusCode(Throwable t) {
	Throwable current = t;
	for (int depth = 0; current != null && depth < MAX_CAUSE_DEPTH; depth++) {
	if (current instanceof StatusRuntimeException) {
	return ((StatusRuntimeException) current).getStatus().getCode();
	}
	if (current instanceof StatusException) {
	return ((StatusException) current).getStatus().getCode();
	}
	if (current instanceof CancellationException) {
	return Status.Code.CANCELLED;
	}
	Throwable next = current.getCause();
	if (next == current) {
	break; // self-referential chain
	}
	current = next;
	}
	return null;
	}

[gemini-code-assist]

To prevent a potential NullPointerException if cause is null, we should add a null check at the beginning of describeUnrecognized before calling cause.getMessage().

  private static String describeUnrecognized(@Nullable Throwable cause) {
    if (cause == null) {
      return "Session operation failed with a null error; reporting UNKNOWN.";
    }
    StringBuilder chain = new StringBuilder();
    Throwable current = cause;

[mutianf]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances exception translation in DivertingUnaryCallable by walking the cause chain of throwables to extract or infer gRPC status codes, and adds comprehensive tests to validate these mappings. The review feedback suggests a performance optimization to pre-compute the statusless error message once, avoiding redundant cause-chain traversals during logging and exception creation.

[gemini-code-assist]

The describeStatusless method is called multiple times for the same exception (once for logging and once for creating the exception). We can avoid redundant string construction and cause-chain traversal by computing the message once and passing it to reportStatusless.

    Status.Code inferred = classifyStatuslessCause(cause);
    String message = describeStatusless(cause, inferred);
    reportStatusless(message, cause);
    return ApiExceptionFactory.createException(
        message, e, GrpcStatusCode.of(inferred), false);

[gemini-code-assist]

Update reportStatusless to accept the pre-computed message string directly, avoiding redundant calls to describeStatusless.

Suggested change

	private void reportStatusless(@Nullable Throwable cause, Status.Code reported) {
	if (loggedStatusless.compareAndSet(false, true)) {
	LOGGER.log(Level.WARNING, describeStatusless(cause, reported), cause);
	} else if (LOGGER.isLoggable(Level.FINE)) {
	LOGGER.log(Level.FINE, describeStatusless(cause, reported), cause);
	}
	}
	private void reportStatusless(String message, @Nullable Throwable cause) {
	if (loggedStatusless.compareAndSet(false, true)) {
	LOGGER.log(Level.WARNING, message, cause);
	} else if (LOGGER.isLoggable(Level.FINE)) {
	LOGGER.log(Level.FINE, message, cause);
	}
	}