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

Empty grpc-message header throws an error #1109

Open
gurkohli opened this issue Jun 14, 2022 · 7 comments
Open

Empty grpc-message header throws an error #1109

gurkohli opened this issue Jun 14, 2022 · 7 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@gurkohli
Copy link

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
Copy link
Contributor

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.

@johanbrandhorst johanbrandhorst added the kind/bug Categorizes issue or PR as related to a bug. label Jun 15, 2022
@gurkohli
Copy link
Author

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

@gurkohli
Copy link
Author

@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
Copy link
Contributor

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

@gurkohli
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

2 participants