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

Could we add flags to headers/data event? #149

Closed
xiejiangzhi opened this issue Mar 30, 2019 · 5 comments
Closed

Could we add flags to headers/data event? #149

xiejiangzhi opened this issue Mar 30, 2019 · 5 comments

Comments

@xiejiangzhi
Copy link
Contributor

xiejiangzhi commented Mar 30, 2019

When I forward a gRPC request, I must send 3 frames(headers -> data -> headers).
I think add flags of frame to event of data/headers is a easy way.

How do you think so?
If you think it is ok, will create a PR for it.

@igrigorik
Copy link
Owner

When I forward a gRPC request, I must send 3 frames(headers -> data -> headers).

As in, you need to send a trailer? You're not allowed to interleave headers and data frames within a stream.

I think add flags of frame to event of data/headers is a easy way.

How or why would this address what you're after?

@xiejiangzhi
Copy link
Contributor Author

@igrigorik According to the GRPC spec
https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses

HEADERS (flags = END_HEADERS)
:status = 200
grpc-encoding = gzip
content-type = application/grpc+proto

DATA
<Length-Prefixed Message>

HEADERS (flags = END_STREAM, END_HEADERS)
grpc-status = 0 # OK
trace-proto-bin = jher831yy13JHy3hc

Because I need know headers & data of request, check all frame will take more time.

remote_server_stream.on(:headers) do |headers|
  user_stream.headers(headers, end_stream: stream_is_end, end_header: headers_is_end?)
end

remote_server_stream.on(:data) do |data|
  user_stream.data(data, end_stream: stream_is_end?)
end

Other way is collecting all headers & data to an array, and send them when remote_server_stream close. Then, I cannot forward them real-time.

Because I didn't take more time to read code & understand http2, I didn't process it by frame_received event.

So which way do you think is better?

@igrigorik
Copy link
Owner

You definitely shouldn't buffer. What you're doing by listening to on(:headers) is correct: that will fire when you first receive the headers, and it will fire again when the trailer is received. For the latter case, you can check stream status stream.closed? or stream.status from inside of the headers callback. I don't think you need to get the flags. Does that make sense?

@xiejiangzhi
Copy link
Contributor Author

xiejiangzhi commented Apr 4, 2019

Yes, it's right if the just for gRPC. But I want to forward all h2 request. I think stream.close cannot help me to do it when a normal stream that only send one header.
And I ave checked the stream.closed?, it always be false in on(:headers) and on(:data)

conn.on(:frame_received) { |f| puts " <- #{f.inspect}" }
stream.on(:headers) do |h|
  headers.push(*h)
  puts " -- stream.closed? #{stream.closed?}"
end

stream.on(:data) do |bytes|
  body << bytes
  puts " -- stream.closed? #{stream.closed?}"
end

stream.on(:close) do
  puts '-' * 80
  puts " -- stream.closed? #{stream.closed?}"
  puts headers.inspect
  puts body.inspect
end
 <- {:length=>19, :type=>:headers, :flags=>[:end_headers], :stream=>1, :payload=>"\x88_\x90\x1Du\xD0b\r&=LMed\xFFu\xD8t\x9F"}
 -- stream.closed? false
 <- {:length=>12, :type=>:data, :flags=>[], :stream=>1, :payload=>"\x00\x00\x00\x00\a\n\x05reply"}
 -- stream.closed? false
 <- {:length=>12, :type=>:headers, :flags=>[:end_stream, :end_headers], :stream=>1, :payload=>"@\x88\x9A\xCA\xC8\xB2\x124\xDA\x8F\x010"}
 -- stream.closed? false
--------------------------------------------------------------------------------
 -- stream.closed? true
[[":status", "200"], ["content-type", "application/grpc+proto"], ["grpc-status", "0"]]
"\u0000\u0000\u0000\u0000\a\n\u0005reply"

@xiejiangzhi
Copy link
Contributor Author

xiejiangzhi commented Apr 4, 2019

If I cannot get the flag of current frame from on(:headers) and on(:data), only two choices for me.
one is to put headers and data to a buffer, and send them when stream close, the other is to handle the on(:frame_received) and forward headers/data

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

No branches or pull requests

2 participants