search

connectrpc / connect-es

The TypeScript implementation of Connect: Protobuf RPC that works.
https://connectrpc.com/
Apache License 2.0
1.32k stars 75 forks source link

Inconsistent debug Field Type in Error Details After connect-go v1.15.0 Update #1095

Closed wangyuehong closed 6 days ago

wangyuehong commented 3 months ago

Describe the bug

After https://github.com/connectrpc/connect-go/pull/688 was merged into connect-go, the debug field in error details is not consistently an object but can be any JSON type.

Error JSON produced by connect-go v1.14.0 and earlier:

{
    "code": "invalid_argument",
    "message": "bad sentence: xxx",
    "details": [
        {
            "type": "google.protobuf.StringValue",
            "value": "Cg5zZXJ2ZXIgZGV0YWlscw",
            "debug": {
                "@type": "type.googleapis.com/google.protobuf.StringValue",
                "value": "server details"
            }
        }
    ]
}

The same error JSON from connect-go v1.15.0:

{
    "code": "invalid_argument",
    "message": "bad sentence: xxx",
    "details": [
        {
            "type": "google.protobuf.StringValue",
            "value": "Cg5zZXJ2ZXIgZGV0YWlscw",
            "debug": "server details"
        }
    ]
}

The current code in connect-es expects the debug field to be an object, so the new error JSON format cannot be handled after connect-go v1.15.0.

https://github.com/connectrpc/connect-es/blob/e5804707c64a2a1c3cfc6a12753047ee274215e3/packages/connect/src/protocol-connect/error-json.ts#L65

To Reproduce Return the following error in a connect-go server:

err := connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("bad sentence: %s", "xxx"))
errDetail, _ := connect.NewErrorDetail(&wrapperspb.StringValue{Value: "server details"})
err.AddDetail(errDetail)

Expected error:

ConnectError: [invalid_argument] bad sentence: xxx

Actual error:

ConnectError: [invalid_argument] HTTP 400

Environment (please complete the following information):

Additional context This code might fix the issue, but I am not familiar with TypeScript, so I am just reporting it:

         typeof detail.type != "string" ||
-        typeof detail.value != "string" ||
-        ("debug" in detail && typeof detail.debug != "object")
+        typeof detail.value != "string"
       ) {
srikrsna-buf commented 3 months ago

Thank you for the detailed report. @jhump should this be added to conformance suite?

jhump commented 3 months ago

@jhump should this be added to conformance suite?

That's probably a good idea. The well-known types seem to always be causing problems :(