-
Notifications
You must be signed in to change notification settings - Fork 557
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
Add a failing spec demonstrating a bug in the em-http-request adapter. #185
Conversation
When a request is made to a URL that returns a 3xx response and the :redirects option is set, the globally_stub_request/after_request hooks are not paired properly. Both hooks should receive the original request and the redirect-following request. This spec should probably be re-written to use the local webmock server, but I couldn't figure out how to get it to conditionally send a redirect response since it writes directly to the socket and doesn't (as far as I can tell) have the request info available in that scope...so there's not easy way to have it send a different response for different requests :(. See myronmarston/vcr#171 for the original VCR issue that caused me to investigate this bug.
Hi Myron, Thanks for the test. I don't have much time now, but I'll try to fix it at some point. |
@bblimke -- thanks for all your work on WebMock. It'd be nice to get this fixed at some point but it's not a blocker for VCR or anything like that. @markiz -- As a VCR/WebMock/em-http-request user (and the source of the original bug report on the VCR issue tracker), it'd be great if you could jump in here and try to fix this yourself, if you have the time. |
Understood. I will try sometime around the weekend then. |
…ns a 3xx response and the :redirects option is set, the globally_stub_request/after_request hooks are now fired for the original request and the redirect-following request.
@myronmarston ok, it should work fine now, but I'm not sure it's now compatible with the workaround you did in VCR. Please test and let me know. |
VCR's specs are all green. Thanks for fixing this! |
Mmm I have a similar issue: A request stubbed as 302 by webmock doesn't get followed by But if I try to stub the two requests, after the first is done (the 302) I'm using |
I wrote a small test to reproduce the issue, here it is: https://gist.github.com/4294952 |
@jarthod WebMock doesn't currently follow redirects returned from stubbed responses in none of the supported http clients. I suggest opening a new issue for that as this is a different issue. |
Ok, I openned a new issue: #237 |
When a request is made to a URL that returns a 3xx response and the
:redirects option is set, the globally_stub_request/after_request
hooks are not paired properly. Both hooks should receive the original
request and the redirect-following request.
This spec should probably be re-written to use the local webmock
server, but I couldn't figure out how to get it to conditionally
send a redirect response since it writes directly to the socket
and doesn't (as far as I can tell) have the request info available
in that scope...so there's not easy way to have it send a different
response for different requests :(.
See myronmarston/vcr#171 for the original VCR issue that caused
me to investigate this bug.
@bblimke -- let me know if I can help with this in anyway. I'm pretty useless when it comes to em-http issues since I've never used it but I'll see what I can do. Also, there's some other weirdness here...the spec for the
after_request
hook fails in different ways, different times it is run...sometimes theafter_request
hook is invoked twice and sometimes it's invoked only once. Either way, it never gets invoked with the https redirect URL.