feat(storage): log additional bytes received from GCS in read path

[nidhiii-27]

Fixes bug 475824752. Tracks the number of bytes received from the transport layer and emits a warning log if GCS sends more bytes than requested by the user during explicit range requests.

[Generated-by: AI]

[gemini-code-assist]

Code Review

This pull request adds logging warnings to ApiaryUnbufferedReadableByteChannel and GapicUnbufferedReadableByteChannel when more bytes are received from GCS than requested. The review feedback points out that the logging logic in the close() methods of both classes is not exception-safe. If a runtime exception occurs during logging, it could prevent critical resource cleanup and state updates from executing, potentially causing resource leaks. The reviewer recommends wrapping the logging logic in a try-finally block to ensure cleanup is always executed safely.

[gemini-code-assist]

The logging logic inside close() is not exception-safe. If any runtime exception occurs during logging, it will prevent the subsequent sbc.close() from being executed, resulting in a resource leak. Instead of adding defensive null checks (which should be avoided for values guaranteed to be non-null by design), wrap the logging block in a try block and place the resource cleanup (sbc.close()) in a finally block to ensure it is always executed safely.

Suggested change

	long requestedLength = apiaryReadRequest.getByteRangeSpec().length();
	if (requestedLength >= 0
	&& requestedLength < ByteRangeSpec.EFFECTIVE_INFINITY
	&& totalBytesReadFromNetwork > requestedLength) {
	java.util.logging.Logger.getLogger(ApiaryUnbufferedReadableByteChannel.class.getName())
	.warning(
	String.format(
	"storage: received %d more bytes than requested from GCS for bucket '%s', object '%s'",
	totalBytesReadFromNetwork - requestedLength,
	apiaryReadRequest.getObject().getBucket(),
	apiaryReadRequest.getObject().getName()));
	}
	try {
	long requestedLength = apiaryReadRequest.getByteRangeSpec().length();
	if (requestedLength >= 0
	&& requestedLength < ByteRangeSpec.EFFECTIVE_INFINITY
	&& totalBytesReadFromNetwork > requestedLength) {
	java.util.logging.Logger.getLogger(ApiaryUnbufferedReadableByteChannel.class.getName())
	.warning(
	String.format(
	"storage: received %d more bytes than requested from GCS for bucket '%s', object '%s'",
	totalBytesReadFromNetwork - requestedLength,
	apiaryReadRequest.getObject().getBucket(),
	apiaryReadRequest.getObject().getName()));
	}
	} finally {
	sbc.close();
	}

References

1. The implementation must be exception-safe to prevent resource leaks, meaning all opened resources should be closed even if exceptions occur during their creation or closing.
2. Avoid adding defensive null checks for values that are guaranteed to be non-null by design, as this can hide invariant breaks.

[gemini-code-assist]

Similarly, the logging logic inside close() is not exception-safe. If any runtime exception occurs during logging, it will prevent the subsequent cleanup code (setting open = false and cleaning up leftovers) from being executed. Instead of adding defensive null checks, wrap the logging block in a try block and place the cleanup logic in a finally block to ensure it is always executed safely.

    try {
      long readLimit = req.getReadLimit();
      long receivedBytes = fetchOffset.get() - req.getReadOffset();
      if (readLimit > 0 && receivedBytes > readLimit) {
        java.util.logging.Logger.getLogger(GapicUnbufferedReadableByteChannel.class.getName())
            .warning(
                String.format(
                    "storage: received %d more bytes than requested from GCS for bucket '%s', object '%s'",
                    receivedBytes - readLimit,
                    req.getBucket(),
                    req.getObject()));
      }
    } finally {
      open = false;
      leftovers = null;
    }

References

1. The implementation must be exception-safe to prevent resource leaks, meaning all opened resources should be closed even if exceptions occur during their creation or closing.
2. Avoid adding defensive null checks for values that are guaranteed to be non-null by design, as this can hide invariant breaks.

[nidhiii-27]

Addressed all PR review comments.