CodingAleCR / http_interceptor

A lightweight, simple plugin that allows you to intercept request and response objects and modify them if desired.
MIT License
134 stars 67 forks source link

Make it work with Client.send() #48

Closed gregor-vi closed 2 years ago

gregor-vi commented 3 years ago

Currently it doesn't seem to work whenever using the http client's 'send()' method, which makes the interceptor useless in those cases.

issue-label-bot[bot] commented 3 years ago

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.57. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

CodingAleCR commented 3 years ago

Hi yes, the send method is not currently supported, it is included more as an escape hatch. I will be adding it as soon as I get the time to do it since it's top priority.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

shyndman commented 3 years ago

Hi @CodingAleCR,

I found myself in need of something like this, and I put together a tiny project demonstrating an approach to getting send intercepting reasonably. Nothing is too polished, but I wanted to hear your opinion.

https://github.com/shyndman/http-recorder

(Also, thanks for the library — it's super useful)

CodingAleCR commented 3 years ago

Hey @shyndman!

Thanks a lot, I will take a look at it and come back to you. I've been playing around with making send work via cloning the request but have seemed to hit a wall when copying StreamedRequests, I would love it if I can get it working before 1.0.0 gets released, your repository will definitely help me out a ton as I am sure I will learn a lot.

shyndman commented 3 years ago

I haven't actually tried with StreamedRequest...but its documentation is slightly worrying. I'm going to try it out and get back.

CodingAleCR commented 3 years ago

I will push my code to a branch so you can see where I'm going with this, any feedback is welcomed. Still haven't gotten around to checking your repository, but will probably take a look during the weekend. Cheers!

shyndman commented 3 years ago

So turns out my solution works fine for StreamedRequests. Woot! There are some bugs in the recorder, but I'll get them sorted.

shyndman commented 3 years ago

Also works for multipart requests, and completely avoids all that annoying finalization stuff they have in there.

Works on the basis of a ByteStream subclass that wraps an inner stream, caches its contents, and can hand out a single-subscriber stream to anyone who calls listen(). Those streams have the entire sequence of bytes replayed, so nothing gets lost.

There are memory implications if you keep them around, but it's perfect what I need. And because it only uses single-subscriber streams, there's no reason that they'll be staying in memory unexpectedly.

BUT, I would much rather be using your library, and that's why I'm here. :)

shyndman commented 3 years ago

Pushed up the latest, which includes a demonstration of StreamedRequest interception in the example.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

II11II commented 3 years ago

Any news?

CodingAleCR commented 3 years ago

Unfortunately it is not complete yet, as it involves quite a bit of refactoring I'm going piece by piece to make sure it does not break anything.

shyndman commented 3 years ago

Worth mentioning that I solidified this pattern (for a specific use case) here: https://github.com/madewithfelt/betamax

It works great, and I'm pretty sure I fixed a couple tricky bugs during the transition from experiment to package.

shyndman commented 3 years ago

Oh, and worth mentioning, I'd love to swap out the lower level code for your stuff if it was available.

Happy to help to, if you want it.

CodingAleCR commented 3 years ago

Help is always welcomed @shyndman so we could definitely collab on it 💪🏼

CodingAleCR commented 3 years ago

@all-contributors add @gregor-vi for ideas

allcontributors[bot] commented 3 years ago

@CodingAleCR

Could not find the user gregor-vi, on github.

CodingAleCR commented 3 years ago

@all-contributors add @shyndman for ideas

allcontributors[bot] commented 3 years ago

@CodingAleCR

I've put up a pull request to add @shyndman! :tada:

CodingAleCR commented 3 years ago

@all-contributors add @II11II for ideas

allcontributors[bot] commented 3 years ago

@CodingAleCR

I've put up a pull request to add @II11II! :tada:

kiaxseventh commented 3 years ago

@CodingAleCR any news? client.send() method are not shipped via http_interceptor Therefore, RetryPolicy and InterceptorContract do not work i have problem for upload file with authentication

CodingAleCR commented 3 years ago

Still working on it, I'm having issues when trying out the implementation with different request types like Multipart or Streamed. I'm sorry for not having better news 😔

shyndman commented 3 years ago

Trying out the Betamax implementation? What are you running into?

vandres commented 3 years ago

I also would love to see a solution for that. Using a library, which is dependent on send() and I would like to add some headers.

CodingAleCR commented 3 years ago

Trying out the Betamax implementation? What are you running into?

The thing with it is that it's dependant on dart:io, so I can't fully use your approach since we would loose web and desktop support, I have a similar approach but still in development.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Mathew-Smith commented 2 years ago

Would be great to see support for client.send(). Is it still in the pipeline?

irvine5k commented 1 year ago

any news?