Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Status package is tightly coupled with golang/protobuf #1885

Closed
johanbrandhorst opened this issue Feb 26, 2018 · 1 comment
Closed

Status package is tightly coupled with golang/protobuf #1885

johanbrandhorst opened this issue Feb 26, 2018 · 1 comment

Comments

@johanbrandhorst
Copy link
Contributor

@johanbrandhorst johanbrandhorst commented Feb 26, 2018

Please answer these questions before submitting your issue.

What version of gRPC are you using?

gRPC 1.10

What version of Go are you using (go version)?

go version go1.10 linux/amd64

What operating system (Linux, Windows, …) and version?

$ uname -a
Linux johan-x1 4.15.3-2-ARCH #1 SMP PREEMPT Thu Feb 15 00:13:49 UTC 2018 x86_64 GNU/Linux

What did you do?

I tried using the grpc/status library with gogo/protobuf.

What did you expect to see?

I expected to be able to use gogo/protobuf registered types with the gRPC status details. The details is a slice of google.protobuf.Any messages.

What did you see instead?

I found that the gRPC status library imports github.com/golang/protobuf. I also found it is tightly coupled to this implementation for unmarshalling of google.protobuf.Any messages, making use of the internal type registry.

In much the same vein as #1873, this presents a problem for users of gogo/protobuf, essentially forcing them to register their types with both backends. In the status library this comes down to the use of ptypes.MarshalAny and ptypes.UnmarshalAny. Both of these could perhaps be replaced with some sort of interface for accomplishing this, and the proto.Message type could be maintained as that is implemented by all gogo generated types as well.

In general, both #1873 and this issue come down to the same issues mentioned by @dsnet in gogo/protobuf#386 (comment).

I think it's worthwhile for Go gRPC to maintain a distance from golang/protobuf since the stated goal of gRPC is to be transport agnostic, though I'm not sure how that applies to the grpc-status-details-bin trailer. Is there a spec somewhere detailing this? It does not seem to be part of the gRPC wire protocol.

@dfawley
Copy link
Contributor

@dfawley dfawley commented Mar 1, 2018

Note that it's possible to create your own Status proto message and use it directly: https://godoc.org/google.golang.org/grpc/status#FromProto. This means you don't need to use the ptypes functions at all, even when you have details to include with your status. We'll still serialize the status using proto.Marshal() when writing to the wire, however.

Unfortunately, grpc has always been coupled with proto, which is why status was implemented this way. I'm not sure if there are any acceptable solutions here that both remove the tight coupling and also maintain backward compatibility. I'm happy to hear about how we might fix this without needing a version 2.0, but our team doesn't have the bandwidth to take this on for some time.

johanbrandhorst added a commit to johanbrandhorst/grpc-go that referenced this issue Mar 18, 2018
Embues the status package with the ability to create statuses
from generic errors that implement a special interface.

This was designed with the github.com/gogo/protobuf project in mind,
but is implemented in a fashion that makes it accessible to arbitrary
payloads.

Fixes grpc#1885.
johanbrandhorst added a commit to johanbrandhorst/grpc-go that referenced this issue Mar 18, 2018
Embues the status package with the ability to create statuses
from generic errors that implement a special interface.

This was designed with the github.com/gogo/protobuf project in mind,
but is implemented in a fashion that makes it accessible to arbitrary
payloads.

Fixes grpc#1885.
johanbrandhorst added a commit to johanbrandhorst/grpc-go that referenced this issue Mar 18, 2018
Embues the status package with the ability to create statuses
from generic errors that implement a special interface.

This was designed with the github.com/gogo/protobuf project in mind,
but is implemented in a fashion that makes it accessible to arbitrary
payloads.

Fixes grpc#1885.
johanbrandhorst added a commit to johanbrandhorst/grpc-go that referenced this issue Mar 18, 2018
Embues the status package with the ability to create statuses
from generic errors that implement a special interface.

This was designed with the github.com/gogo/protobuf project in mind,
but is implemented in a fashion that makes it accessible to arbitrary
payloads.

Fixes grpc#1885.
johanbrandhorst added a commit to johanbrandhorst/grpc-go that referenced this issue Mar 18, 2018
Embues the status package with the ability to create statuses
from generic errors that implement a special interface.

This was designed with the github.com/gogo/protobuf project in mind,
but is implemented in a fashion that makes it accessible to arbitrary
payloads.

Fixes grpc#1885.
johanbrandhorst added a commit to johanbrandhorst/grpc-go that referenced this issue Mar 24, 2018
Embues the status package with the ability to create statuses
from generic errors that implement a special interface.

This was designed with the github.com/gogo/protobuf project in mind,
but is implemented in a fashion that makes it accessible to arbitrary
payloads.

Fixes grpc#1885.
johanbrandhorst added a commit to johanbrandhorst/grpc-go that referenced this issue Mar 26, 2018
Embues the status package with the ability to create statuses
from generic errors that implement the interface:

type StatusError interface {
    Status() *Status
}

This was designed with the github.com/gogo/protobuf project in mind,
but is implemented in a fashion that makes it accessible to arbitrary
payloads.

Fixes grpc#1885.
dfawley added a commit that referenced this issue Mar 26, 2018
…1927)

Embues the status package with the ability to create statuses
from generic errors that implement the interface:

type StatusError interface {
    Status() *Status
}

This was designed with the github.com/gogo/protobuf project in mind,
but is implemented in a fashion that makes it accessible to arbitrary
payloads.

Fixes #1885.
@lock lock bot locked as resolved and limited conversation to collaborators Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.