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

Fix cached status code for ibrowse #57

Merged
merged 2 commits into from
Apr 27, 2016

Conversation

vanstee
Copy link
Contributor

@vanstee vanstee commented Apr 25, 2016

Before storing a response in the cache, convert_to_string is called which, for ibrowse, converts the status code from a char list to an integer. This is usually fine, unless you're making the same api request when recording a single cassette. On the second request, the response is grabbed from the cache and passed to ibrowse to return to the user. But, since the stored response has been converted from it's original tuple of char lists, ibrowse returns a response HTTPotion is not expecting, an integer status_code instead of a char list. This might be better solved by changing the representation of the response in the cache, but this fixed the immediate bug we hit in our tests.

vanstee added 2 commits March 28, 2016 16:23
Currently, for the ibrowse adapter, status codes are converted back into
char lists when loaded from a json file, but not when they're pulled
back out of the cache. If you cache a request and then immediately try
to use it (without writing it and reading it from a file) the status
code we get back from the ibrowse call will be an integer. This fixes
that and transforms all status codes back into char lists when pulled
directly from the cache.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 93.137% when pulling f64a59a on operable:fix-cached-status-code into 0ae8a0f on parroty:master.

@parroty parroty merged commit 6980a01 into parroty:master Apr 27, 2016
@parroty
Copy link
Owner

parroty commented Apr 27, 2016

Thanks!

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