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

[bug] RPC api forwards http headers incorrectly #285

Closed
seeflood opened this issue Oct 26, 2021 · 2 comments · Fixed by #291
Closed

[bug] RPC api forwards http headers incorrectly #285

seeflood opened this issue Oct 26, 2021 · 2 comments · Fixed by #291
Assignees

Comments

@seeflood
Copy link
Member

seeflood commented Oct 26, 2021

What happened:
When invoking RPC api using typescript sdk written in #262 ,the client got an error when parsing the response frame.

Error: 13 INTERNAL: Received RST_STREAM with code 1

image

What you expected to happen:
The typescript sdk invokes RPC api successfully

How to reproduce it (as minimally and precisely as possible):
Follow the README doc in #262

Anything else we need to know?:
There are two bugs:

  1. We found that this bug was caused by directly forwarding the 'Content-Length' header to the client.
    If we modify Layotto's code and make it not to forward this field, the ut will invoke it successfully
    image

  2. Sometimes a different error occurs:

Error: 14 UNAVAILABLE: io: read/write on closed pipe

image

@wenxuwan wenxuwan self-assigned this Oct 27, 2021
@wenxuwan
Copy link
Member

I test golang client, don't have the error, so may be nodejs grpc framework have some different implementation, i will check it and give feedback

@seeflood
Copy link
Member Author

seeflood commented Oct 27, 2021

Maybe we should add a metadata or header field in RPC API response?
There are two considerations:

  1. Make the protocal transparent. Users do not need to pay attention to the real protocol
  2. The rpc api not only supports the transfer to http, but also supports transfer to other rpc protocols; if the headers of other protocols are passed back transparently, they may also encounter field conflicts;
    So adding a field is a solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants