Skip to content

Allow request/response interception on stubs - #232

Merged
ronwsmith merged 2 commits into
oesmith:masterfrom
Jack12816:master
Apr 30, 2018
Merged

ronwsmith merged 2 commits into
oesmith:masterfrom
Jack12816:master

Conversation

@Jack12816

Copy link
Copy Markdown
Contributor

- What is it good for?

This update ships the possibility to intercept requests and responses on the proxy stub method. This is quite handy when you want to modify ugly client requests or just want to modify the response body on certain requests. In the test suite where this feature was implemented, we wanted to intercept responses with a content disposition header and save their content to file. (aka file download) This is required due to the some bugs on Chrome atm. (Cant save PDF files on headless, crash on new tabs of PDF files on non-headless)

- What I did

I added a new pass_request method to the ProxyRequestStub class which makes use of the existing functionality of the proxy handler. Furthermore, I switched the proc execution mechanism from Proc#call to Object#instance_exec to allow a lean syntax on the given Proc. See the examples in the readme. I also added the requested URL and its HTTP method to the list of arguments of the and_return callable. This was proved on the corresponding test. A new test case was added to prove the pass_request mechanism. Unfortunately eventmachine weeps about: (eval):8:in resume': double resume (FiberError)` on the test case. I have to dig into this, or if anybody knows why/how to fix this - tell me. :)

- A picture of a cute animal (not mandatory but encouraged)

images 1

@ronwsmith

Copy link
Copy Markdown
Collaborator

Found this issue, but not sure if the workarounds are applicable: websocket-rails/websocket-rails#191

@Jack12816 Jack12816 changed the title Allow request/response interception with on stubs. Allow request/response interception on stubs Mar 6, 2018
@Jack12816

Copy link
Copy Markdown
Contributor Author

@ronwsmith Thanks for the assistance! I dug into this and found a solution.
The test case contains an inline documentation of the issue.

Hope this is good to merge then? ☺️

@ronwsmith

Copy link
Copy Markdown
Collaborator

Looks good, I'll try to get this in next week.

@ronwsmith
ronwsmith merged commit ee51505 into oesmith:master Apr 30, 2018
@ronwsmith

Copy link
Copy Markdown
Collaborator

Released in 1.1.0

@AlanFoster

Copy link
Copy Markdown
Contributor

Hey; I've just upgraded to v1.1.0 and this is a breaking change for our code base.

Just cross referencing this issue I've created:

#241

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.

3 participants