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

Response chain #1428

Open
thomas-zahner opened this issue May 15, 2024 · 1 comment
Open

Response chain #1428

thomas-zahner opened this issue May 15, 2024 · 1 comment
Labels
enhancement New feature or request question Further information is requested

Comments

@thomas-zahner
Copy link
Member

thomas-zahner commented May 15, 2024

The Client currently has a plugin_request_chain but no plugin_response_chain. It might make sense to add a response chain.

In the following snippet we can see how the request chain is traversed and we get the status. handle_github is the perfect candidate for an element in the internal default response chain.

let status = ClientRequestChains::new(vec![&self.plugin_request_chain, default_chain])
    .traverse(request)
    .await;

self.handle_github(status, uri).await

The question is how we define the ResponseChain type. Do we want to include the response body? This way plugin users could handle complicated use cases based on the response body.

pub type ResponseChain = Chain<Status, Status>;

or

pub type ResponseChain = Chain<(Body, Status), Status>;

@thomas-zahner thomas-zahner added enhancement New feature or request question Further information is requested labels May 16, 2024
@mre
Copy link
Member

mre commented Oct 8, 2024

Yes, I like the symmetry between plugin_request_chain and plugin_response_chain.
Regarding the proposed ResponseChain type, I believe including the response body would offer more versatility:

pub type ResponseChain = Chain<(Body, Status), Status>;

This approach would allow plugin users to handle complex scenarios based on both the status and the response body. It provides maximum flexibility while still allowing simpler plugins to ignore the body if not needed.

Some potential benefits of a response chain:

  1. Custom error handling or retries based on specific response contents
  2. Response transformation or sanitization
  3. Logging or metrics collection on responses

However, we should consider the performance impact of passing large response bodies through the chain.

Before proceeding, it might be helpful to gather more use cases from the community to ensure this addition would be broadly useful and prototype the change to assess its impact on performance and existing code

Would you like to proceed with prototyping this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants