diff --git a/go/adbc/adbc.go b/go/adbc/adbc.go index 636320fa31..6920195b84 100644 --- a/go/adbc/adbc.go +++ b/go/adbc/adbc.go @@ -85,7 +85,7 @@ func (d *ProtobufErrorDetail) Serialize() ([]byte, error) { return proto.Marshal(any) } -// ProtobufErrorDetail is an ErrorDetail backed by a human-readable string. +// TextErrorDetail is an ErrorDetail backed by a human-readable string. type TextErrorDetail struct { Name string Detail string @@ -100,7 +100,7 @@ func (d *TextErrorDetail) Serialize() ([]byte, error) { return []byte(d.Detail), nil } -// ProtobufErrorDetail is an ErrorDetail backed by a binary payload. +// BinaryErrorDetail is an ErrorDetail backed by a binary payload. type BinaryErrorDetail struct { Name string Detail []byte diff --git a/go/adbc/driver/flightsql/utils.go b/go/adbc/driver/flightsql/utils.go index acd3ca2dea..52824c8d26 100644 --- a/go/adbc/driver/flightsql/utils.go +++ b/go/adbc/driver/flightsql/utils.go @@ -19,6 +19,7 @@ package flightsql import ( "context" + "errors" "fmt" "strings" @@ -30,13 +31,15 @@ import ( "google.golang.org/protobuf/types/known/anypb" ) +const grpcStatusDetailsBin = "grpc-status-details-bin" + func adbcFromFlightStatus(err error, context string, args ...any) error { var header, trailer metadata.MD return adbcFromFlightStatusWithDetails(err, header, trailer, context, args...) } func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, context string, args ...any) error { - if _, ok := err.(adbc.Error); ok { + if ok := errors.Is(err, adbc.Error{}); ok { return err } @@ -84,7 +87,13 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con details := []adbc.ErrorDetail{} for _, detail := range grpcStatus.Proto().Details { - details = append(details, &anyErrorDetail{name: "grpc-status-details-bin", message: detail}) + if detailProto, err := detail.UnmarshalNew(); err != nil { + // if we could decode into proto, use ProtobufErrorDetail + details = append(details, &adbc.ProtobufErrorDetail{Name: grpcStatusDetailsBin, Message: detailProto}) + } else { + // proceed with any, maybe the user knows how to deserialize? + details = append(details, &anyErrorDetail{name: grpcStatusDetailsBin, message: detail}) + } } // XXX(https://github.com/grpc/grpc-go/issues/5485): don't count on @@ -97,7 +106,7 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con case key == "content-type": // Not useful info continue - case key == "grpc-status-details-bin": + case key == grpcStatusDetailsBin: // gRPC library parses this above via grpcStatus.Proto() continue case strings.HasSuffix(key, "-bin"): @@ -107,6 +116,9 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con } default: for _, value := range values { + if strings.HasPrefix(value, "Grpc-") { + continue + } details = append(details, &adbc.TextErrorDetail{Name: key, Detail: value}) } } @@ -116,7 +128,7 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con case key == "content-type": // Not useful info continue - case key == "grpc-status-details-bin": + case key == grpcStatusDetailsBin: // gRPC library parses this above via grpcStatus.Proto() continue case strings.HasSuffix(key, "-bin"): @@ -126,6 +138,9 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con } default: for _, value := range values { + if strings.HasPrefix(value, "Grpc-") { + continue + } details = append(details, &adbc.TextErrorDetail{Name: key, Detail: value}) } } diff --git a/python/adbc_driver_flightsql/tests/test_errors.py b/python/adbc_driver_flightsql/tests/test_errors.py index d572589aee..e90d79f2ba 100644 --- a/python/adbc_driver_flightsql/tests/test_errors.py +++ b/python/adbc_driver_flightsql/tests/test_errors.py @@ -21,7 +21,6 @@ import threading import time -import google.protobuf.any_pb2 as any_pb2 import google.protobuf.wrappers_pb2 as wrappers_pb2 import pytest @@ -30,10 +29,8 @@ def assert_detail(e): # Check that the expected error details are present found = set() for _, detail in e.details: - anyproto = any_pb2.Any() - anyproto.ParseFromString(detail) string = wrappers_pb2.StringValue() - anyproto.Unpack(string) + string.ParseFromString(detail) found.add(string.value) assert found == {"detail1", "detail2"} diff --git a/python/adbc_driver_flightsql/tests/test_incremental.py b/python/adbc_driver_flightsql/tests/test_incremental.py index 285c18a831..2088da8a00 100644 --- a/python/adbc_driver_flightsql/tests/test_incremental.py +++ b/python/adbc_driver_flightsql/tests/test_incremental.py @@ -18,7 +18,6 @@ import re import threading -import google.protobuf.any_pb2 as any_pb2 import google.protobuf.wrappers_pb2 as wrappers_pb2 import pyarrow import pyarrow.flight @@ -46,10 +45,8 @@ def test_incremental_error(test_dbapi) -> None: found = set() for _, detail in exc_info.value.details: - anyproto = any_pb2.Any() - anyproto.ParseFromString(detail) string = wrappers_pb2.StringValue() - anyproto.Unpack(string) + string.ParseFromString(detail) found.add(string.value) assert found == {"detail1", "detail2"}