search

improbable-eng / grpc-web

gRPC Web implementation for Golang and TypeScript
Apache License 2.0
4.39k stars 436 forks source link

Empty grpc-message header throws an error #1109

Open gurkohli opened 2 years ago

gurkohli commented 2 years ago

Versions of relevant software used 0.15.0

What happened Error thrown because of empty grpc-message header

What you expected to happen Request completes successfully because empty grpc-message comes with OK status

How to reproduce it (as minimally and precisely as possible): Have server send a response with grpc-message header as empty string

Full logs to relevant components n.getHeaderValues(...).forEach is not a function

Anything else we need to know The minified version of grpc-web-client.js, inside the function getHeaderValues does a truthy check on the header as such

n && "string" == typeof n ? [n] : n

This check fails when n is an empty string, as in the case of grpc-message. This causes the function to return undefined. Calling forEach on the return value throws the error

johanbrandhorst commented 2 years ago

Hi, thanks for the bug report. Would you be willing to submit a fix for this? I'm more of a backend engineer and wouldn't know where to fix this.

gurkohli commented 2 years ago

Hey, sure I will work on the fix and submit a PR soon. Thank you.

gurkohli commented 2 years ago

@johanbrandhorst I have a PR open in improbable-eng/js-browser-headers to fix this. Would you have or know someone with privileges to review it?

johanbrandhorst commented 2 years ago

I don't have these privileges. Maybe @MarcusLongmuir or @jonnyreeves does?

gurkohli commented 2 years ago

@johanbrandhorst We're blocked until this PR is merged, would you be able to push someone to review this considering it's the same organization or have some sort of ETA on when it can be merged? I see open PRs from long ago in that branch, looks like it's dead.

johanbrandhorst commented 2 years ago

I'm not part of Improbable unfortunately, I just signed up to maintain this specific repository. I've tagged the people who might be able to help already. As you might have noticed, this tech stack is a bit poorly maintained. I would recommend using one of the other grpc-web clients if you can:

gurkohli commented 2 years ago

Ah okay I understand. I guess we would have to look for alternate solutions if this PR doesn’t get merged soon. Thanks for all your help!