search

twitchtv / twirp

A simple RPC framework with protobuf service definitions
https://twitchtv.github.io/twirp/docs/intro.html
Apache License 2.0
7.14k stars 327 forks source link

Multiple WithHTTPRequestHeaders #380

Closed RobertBolender closed 5 months ago

RobertBolender commented 1 year ago

Hello! I'm attempting to add two different custom HTTP headers to a Twirp request in two different locations in my codebase, and I'm realizing that the server only sees the outermost context layer with the custom header, the last call to WithHTTPRequestHeaders.

Would you be open to a PR that would either update the existing WithHTTPRequestHeaders to merge new headers with existing custom headers (if present)? Or if you feel that would be too much of a breaking change, would you be open to a PR that adds a new separate AddHTTPRequestHeaders method with that behavior?

https://github.com/twitchtv/twirp/blob/206451d552e13701fa1111430aa81235029cb329/context.go#L69

I would expect that I would mimic the existing SetHTTPResponseHeader and AddHTTPResponseHeader, I would like that same behavior we have available for the Response on the Request side as well.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 60 days with no activity. Remove stale label / comment or this will be closed in 5 days

github-actions[bot] commented 12 months ago

This issue is stale because it has been open 60 days with no activity. Remove stale label / comment or this will be closed in 5 days

howsleya commented 8 months ago

We would be open to a PR @RobertBolender. We want to keep the existing behavior as well. So maybe a AddHTTPRequestHeadersWithMerged method?

github-actions[bot] commented 6 months ago

This issue is stale because it has been open 60 days with no activity. Remove stale label / comment or this will be closed in 5 days