Add failing spec demonstrating em-http-request bug. #181

Merged
merged 2 commits into from Aug 26, 2012

Conversation

Projects
None yet
3 participants
@myronmarston
Collaborator

myronmarston commented May 15, 2012

@bblimke -- this is a demonstration of a bug reported by a VCR user at myronmarston/vcr#169. I haven't worked with em-http-request at all so I'm not sure how to fix it but I figured I could at least submit a failing spec. Let me know if you have any questions.

@bblimke

This comment has been minimized.

Show comment Hide comment
@bblimke

bblimke May 15, 2012

Owner

Thanks for the spec! I will try to have a look at it this week.

Owner

bblimke commented May 15, 2012

Thanks for the spec! I will try to have a look at it this week.

Add failing spec demonstrating em-http-request bug.
When a response modifying middleware is used with em-http-request, and a real request is made, the middleware can modify the response before the after_request is invoked. This prevents VCR from being able to record the response accurately.  The after_request hook should be invoked before the em-http-request middleware.

Related to myronmarston/vcr#169.
@sent-hil

This comment has been minimized.

Show comment Hide comment
@sent-hil

sent-hil May 16, 2012

Since I can't find another way, commenting here so I'll get notifications.

Since I can't find another way, commenting here so I'll get notifications.

@sent-hil

This comment has been minimized.

Show comment Hide comment
@sent-hil

sent-hil Aug 19, 2012

Hey @bblimke, any updates on this? Thanks!

Hey @bblimke, any updates on this? Thanks!

@myronmarston

This comment has been minimized.

Show comment Hide comment
@myronmarston

myronmarston Aug 19, 2012

Collaborator

@sent-hil -- given that you have a need to see this fix, do you think you could scratch your own itch and take a stab at fixing this? We all (well, most of us) do this in our free time.

Collaborator

myronmarston commented Aug 19, 2012

@sent-hil -- given that you have a need to see this fix, do you think you could scratch your own itch and take a stab at fixing this? We all (well, most of us) do this in our free time.

@bblimke

This comment has been minimized.

Show comment Hide comment
@bblimke

bblimke Aug 20, 2012

Owner

@sent-hil sorry I was quite busy and I haven't had time to look into it. Thanks for raising the issue again.
If noone else will find a solution and send a pull request earlier, I will to have a look at my earliest "free time".

I had a brief look, but the current em-http-request adapter depends on a callback method which is invoked after all
response middleware are executed. It's worth investigation if there is any available callback in em-http-request,
invoked before middleware, that we could attach WebMock to.

Owner

bblimke commented Aug 20, 2012

@sent-hil sorry I was quite busy and I haven't had time to look into it. Thanks for raising the issue again.
If noone else will find a solution and send a pull request earlier, I will to have a look at my earliest "free time".

I had a brief look, but the current em-http-request adapter depends on a callback method which is invoked after all
response middleware are executed. It's worth investigation if there is any available callback in em-http-request,
invoked before middleware, that we could attach WebMock to.

@bblimke bblimke merged commit f40be82 into master Aug 26, 2012

@bblimke

This comment has been minimized.

Show comment Hide comment
@bblimke

bblimke Aug 26, 2012

Owner

I just merged it into master and the spec now passes without any changes done?! :)

@sent-hil can you please confirm it works now?

maybe it's a change in em-http-request 1.0.3.

Owner

bblimke commented Aug 26, 2012

I just merged it into master and the spec now passes without any changes done?! :)

@sent-hil can you please confirm it works now?

maybe it's a change in em-http-request 1.0.3.

@sent-hil

This comment has been minimized.

Show comment Hide comment
@sent-hil

sent-hil Sep 7, 2012

@bblimke it works!

sent-hil commented Sep 7, 2012

@bblimke it works!

@davidbegin davidbegin deleted the em_http_middleware_after_request_bug branch May 10, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment