Skip to content

Conversation

@eric-wang-1990
Copy link
Contributor

Improve Transport Exception Error Messages in C# Apache Driver

This PR enhances the error handling for Thrift transport exceptions in the C# Apache driver by adding a new method FormatTransportExceptionMessage that extracts HTTP status codes from exception messages and provides more user-friendly error messages for common HTTP errors (401, 403, 404).

Proposed changes

  • Added a new FormatTransportExceptionMessage method to extract and format HTTP status codes from Thrift transport exceptions
  • Enhanced error messages for common HTTP status codes (401 Unauthorized, 403 Forbidden, 404 Not Found)
  • Modified the existing FormatExceptionMessage method to use the new formatting function
  • Added necessary imports for Regex and Thrift.Transport

How is this tested

  • The code changes are straightforward and don't require additional tests
  • The error messages will be more informative when users encounter HTTP-related transport issues

@github-actions github-actions bot added this to the ADBC Libraries 19 milestone Jun 24, 2025
@CurtHagenlocher CurtHagenlocher changed the title feat(csharp/src/Drivers) Format thrift error feat(csharp/src/Drivers): Format thrift error Jun 24, 2025
Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've left a few nitpicking comments and a question.

using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Text.RegularExpressions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: sort

internal static string FormatTransportExceptionMessage(Exception exception)
{
var customMsg = "";
// Use ContainsException to robustly find TTransportException or HttpRequestException
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't line up well with what the code is doing.

}
}
}
return $"{customMsg}{Environment.NewLine}{exception.Message}";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newline here is being added even when customMsg is empty. Does that produce the desired experience?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants