fix: preserve trailer-read failure reason when handshake never completes

Sets failure to the InvalidOperationException so the auth error message
reflects that the handshake did not complete, instead of falling back to
the generic "no authorization token" message. Adds unit tests for the
failure-description logic.

Author: krinart

Spice/src/Flight/SpiceFlightClient.cs

@@ -182,7 +182,7 @@ private async Task AuthenticateAsync()
         // Trailers are only readable once the call has completed. Reading them
         // eagerly throws InvalidOperationException on a handshake that failed,
         // which masks the gRPC status that says what actually went wrong.
-        RpcException? failure = null;
+        Exception? failure = null;
         var token = headers.Get("authorization");
         if (token == null)
         {
@@ -191,9 +191,7 @@ private async Task AuthenticateAsync()
 
         if (token == null || _httpClient == null)
         {
-            var reason = failure == null
-                ? "the runtime returned no authorization token."
-                : DescribeRpcFailure(failure);
+            var reason = DescribeAuthFailure(failure);
 
             throw new SpiceException(
                 SpiceStatus.FailedToAuthenticate,
@@ -209,11 +207,11 @@ private async Task AuthenticateAsync()
     /// completed far enough for them to be available.
     /// </summary>
     /// <param name="stream">The handshake call</param>
-    /// <param name="failure">The gRPC failure, when the trailers could not be read</param>
+    /// <param name="failure">The failure that prevented reading the trailers, if any</param>
     /// <returns>The token entry, or null</returns>
-    private static Metadata.Entry? TryGetTrailerToken(
+    internal static Metadata.Entry? TryGetTrailerToken(
         AsyncDuplexStreamingCall<FlightHandshakeRequest, FlightHandshakeResponse> stream,
-        out RpcException? failure)
+        out Exception? failure)
     {
         failure = null;
         try
@@ -225,9 +223,10 @@ private async Task AuthenticateAsync()
             failure = ex;
             return null;
         }
-        catch (InvalidOperationException)
+        catch (InvalidOperationException ex)
         {
             // The handshake never completed, so there are no trailers to read.
+            failure = ex;
             return null;
         }
     }
@@ -243,6 +242,21 @@ private static string DescribeRpcFailure(RpcException ex)
         return $"{ex.StatusCode} - {detail}.";
     }
 
+    /// <summary>
+    /// Renders why the auth token could not be obtained as something a caller can act on.
+    /// </summary>
+    /// <param name="failure">The failure captured while trying to read the token, if any</param>
+    /// <returns>A short description of the failure</returns>
+    internal static string DescribeAuthFailure(Exception? failure)
+    {
+        return failure switch
+        {
+            null => "the runtime returned no authorization token.",
+            RpcException rpcEx => DescribeRpcFailure(rpcEx),
+            _ => "the handshake did not complete before trailers could be read."
+        };
+    }
+
     internal async Task<FlightClientRecordBatchStreamReader> Query(string sql)
     {
         if (string.IsNullOrEmpty(sql))

SpiceTest/FlightAuthFailureTest.cs

@@ -0,0 +1,63 @@
+/*
+Copyright 2024 The Spice.ai OSS Authors
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+*/
+
+using Grpc.Core;
+using NUnit.Framework;
+using Spice.Flight;
+
+namespace SpiceTest;
+
+/// <summary>
+/// Unit tests for how <see cref="SpiceFlightClient"/> describes why a handshake
+/// failed to yield an authorization token.
+/// </summary>
+[TestFixture]
+public class FlightAuthFailureTest
+{
+    [Test]
+    public void NoFailure_ReturnsNoTokenMessage()
+    {
+        var reason = SpiceFlightClient.DescribeAuthFailure(null);
+
+        Assert.That(reason, Is.EqualTo("the runtime returned no authorization token."));
+    }
+
+    [Test]
+    public void RpcFailure_DescribesStatusAndDetail()
+    {
+        var ex = new RpcException(new Status(StatusCode.Unauthenticated, "invalid api key"));
+
+        var reason = SpiceFlightClient.DescribeAuthFailure(ex);
+
+        Assert.That(reason, Is.EqualTo("Unauthenticated - invalid api key."));
+    }
+
+    [Test]
+    public void IncompleteHandshake_ReturnsIncompleteHandshakeMessage()
+    {
+        var ex = new InvalidOperationException("trailers are not available");
+
+        var reason = SpiceFlightClient.DescribeAuthFailure(ex);
+
+        Assert.That(reason, Is.EqualTo("the handshake did not complete before trailers could be read."));
+    }
+}