eric-wang-1990 commented on PR #3558:
URL: https://github.com/apache/arrow-adbc/pull/3558#issuecomment-3400166089
> If the response has e.g. a 401 status with a
`x-thriftserver-error-message` then this will now throw an exception instead of
returning the `HttpResponseMessage` with a status code. Is the consuming code
still able to translate this exception into an ADBC status code
'Unauthenticated`? Or is my premise wrong because there could never be such a
response?
Before the change:
The exception flow is:
1. _httpClient.PostAsync() returns an HttpResponseMessage with status 401
2. The HttpResponseMessage object exists and contains the
x-thriftserver-error-message header
3. .EnsureSuccessStatusCode() is called on that response object
4. .EnsureSuccessStatusCode() throws HttpRequestException with a generic
message like "Response status code does not indicate success: 401
(Unauthorized)"
5. The x-thriftserver-error-message header is lost because
EnsureSuccessStatusCode() doesn't include custom headers in the exception
message
6. The catch (HttpRequestException ex2) block catches it and wraps it in
TTransportException
After the change (with ThriftErrorMessageHandler):
The exception flow is:
1. _httpClient.PostAsync() calls through the handler chain
2. ThriftErrorMessageHandler intercepts the response BEFORE it returns to
THttpTransport
3. ThriftErrorMessageHandler sees status 401 with
x-thriftserver-error-message header
4. ThriftErrorMessageHandler throws HttpRequestException with detailed
message: "Thrift server error: Invalid personal access token (HTTP 401
Unauthorized)"
5. This exception bubbles up through the HttpClient pipeline
6. From THttpTransport's perspective, PostAsync() itself throws the
exception (not EnsureSuccessStatusCode())
7. The catch (HttpRequestException ex2) block catches it and wraps it in
TTransportException
It should still work for 401 mapping to Adbc authenticated exception since
this
[block](https://github.com/apache/arrow-adbc/blob/2b9262b73416a53ab89f533420c6abfa1d16a472/csharp/src/Drivers/Apache/Hive2/HiveServer2Connection.cs#L391)
is checking on the error message.
I'll fix for NET 5.0+ path though since that can check on the status
directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
