Skip to content

handle dict resp with hidden params (extra hdrs)#3

Closed
tanmaykm wants to merge 1 commit intov1.82.1_with_jh_fixesfrom
tan/responseheaders
Closed

handle dict resp with hidden params (extra hdrs)#3
tanmaykm wants to merge 1 commit intov1.82.1_with_jh_fixesfrom
tan/responseheaders

Conversation

@tanmaykm
Copy link
Member

For the pass through route, when the async_post_call_success_hook is called, it is provided with a dict containing the response. If it needs to add response headers, it has to pass it through that. In this change we add a check for dict responses and pick up extra headers from it if that is the case.

For the pass through route, when the `async_post_call_success_hook` is called, it is provided with a dict containing the response. If it needs to add response headers, it has to pass it through that. In this change we add a check for dict responses and pick up extra headers from it if that is the case.
@tanmaykm tanmaykm marked this pull request as ready for review March 17, 2026 00:58
@tanmaykm tanmaykm requested review from lamdor and mortenpi March 17, 2026 00:58
@mortenpi
Copy link
Member

Can you elaborate what the specific issue is that we're fixing here? It seems like you're saying the response is sometimes a dict and sometimes not a dict?

@tanmaykm
Copy link
Member Author

Yeah... so streaming requests do not result in a dict response, while non-streaming requests do. This is for anthropic, and from what I tested. Unfortunately though the streaming requests in anthropic passthrough do not invoke the plugin callback. So there is no way currently for plugin to pass custom headers in streaming responses. But that is probably a separate thing to be fixed.

From what I understand (and what claude informs) there are several type of responses for other providers, and some of them may have a way to set the headers some not. So the code checks what it got and gets headers if possible.

@tanmaykm
Copy link
Member Author

BTW Looks like there might be a better way now. I see this was merged recently: BerriAI#20070. If that works we may not need this. Will try it out.

@tanmaykm
Copy link
Member Author

... and that works! Closing this

@tanmaykm tanmaykm closed this Mar 17, 2026
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 this pull request may close these issues.

2 participants