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

PROTOCOL_ERROR after upgrading gRPC and Bigtable libraries #1247

Closed
garye opened this issue May 18, 2017 · 10 comments
Closed

PROTOCOL_ERROR after upgrading gRPC and Bigtable libraries #1247

garye opened this issue May 18, 2017 · 10 comments

Comments

@garye
Copy link

garye commented May 18, 2017

The issue was reported here: googleapis/google-cloud-go#628

The Bigtable libraries adds a string to the metadata but otherwise doesn't do much with contexts or metadata. Any ideas what could be going on?

@dfawley
Copy link
Member

dfawley commented May 18, 2017

My guess is this is likely #1219. To add the metadata, are you using metadata.FromContext in a client-side interceptor? If so, it will need to be changed to use FromOutgoingContext.

@bbassingthwaite
Copy link

I've tried explicitly to do: metadata.NewOutgoingContext(ctx, nil) with no success either. Or would you expect this not to work?

@menghanl
Copy link
Contributor

Do we know what the string added to metadata by Bigtable is? Does it contain special characters (like newline)?

@dfawley
Copy link
Member

dfawley commented May 18, 2017

That would clear the outgoing metadata. To add an entry to the outgoing metadata, you should do something like this:

md, _ := metadata.FromOutgoingContext(ctx)
md = metadata.Join(md, metadata.New(map[string]string{"key": "value"}))
ctx = metadata.NewOutgoingContext(ctx, md)

@garye
Copy link
Author

garye commented May 18, 2017

@menghanl We're adding a string without special characters or newlines, and we leverage this code:
https://github.com/GoogleCloudPlatform/google-cloud-go/blob/master/bigtable/bigtable.go#L779

Looks like @dfawley has a PR to fix this for us:
https://github.com/GoogleCloudPlatform/google-cloud-go/pull/627/files

What's the status of that?

@bbassingthwaite
Copy link

So if that is the case: Why does context.Background() work but not metadata.NewOutgoingContext(ctx, nil) ?

@dfawley
Copy link
Member

dfawley commented May 18, 2017

It sounds like it's something else in the context, not the metadata, then...

Can you log the context? It may also be helpful if you could provide a reproducible test case so we can dig into it.

@bbassingthwaite
Copy link

bbassingthwaite commented May 18, 2017

So I had tried clearing both incoming and outgoing, but never both. This line is what ended up working for me. No idea why this works, but it does:

newCtx := metadata.NewOutgoingContext(
    metadata.NewIncomingContext(ctx, nil),
    nil,
)

@dfawley
Copy link
Member

dfawley commented May 18, 2017

Almost definitely what's happening here is the client interceptor that adds its metadata is reading your incoming metadata (because it is using metadata.FromContext), appending its value to the metadata there, then using that as the outgoing metadata. This is the main root cause, and that should be fixed by googleapis/google-cloud-go#627.

In addition, it seems there's characters in the incoming metadata that isn't allowed in text-only outgoing metadata, and that's why we're getting the error we're getting. It would be interesting to know the nature of the redacted authorization/authority values (e.g. are there characters in there outside the range of the base64 encoding set?). Also, you said earlier that you have clients that are running both the latest grpc version and older versions, and only the older versions fail. Can you compare the incoming metadata at the server from those two clients and see if you can discern a difference between them?

@dfawley
Copy link
Member

dfawley commented May 22, 2017

The fix to the bigtable client library was merged, so this should be resolved now.

@dfawley dfawley closed this as completed May 22, 2017
@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.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants