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.

Sign up for GitHub

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

Jump to bottom

Save the last response made on StripeObjects #206

Merged
merged 1 commit into from
Oct 22, 2015
Merged

Save the last response made on StripeObjects #206

merged 1 commit into from
Oct 22, 2015

Conversation

kyleconroy
Copy link
Contributor

Supersedes #191

@kyleconroy kyleconroy mentioned this pull request Oct 20, 2015
Closed
@kyleconroy
Copy link
Contributor Author

r? @bkrausz

@brandur
Copy link
Contributor

brandur commented Oct 20, 2015

Nice! +1. Maybe one missing piece is that it might be worth adding tests on ApiResource to verify the new behavior there.

Another slightly more general question: if getting at the request ID is the main goal here, should we bother exposing the entire response? I guess I could see it either way.

@kyleconroy
Copy link
Contributor Author

@brandur while the impetus for this change was accessing the Request-Id, I think the real request is having access to the response. I didn't want to special case Request-Id and then have someone come back and ask for response status or another header (such as Stripe version).

@kjc-stripe
Save the last API response to StripeObjects.
48191cb 
Calling `getLastResponse` will return the last API response made by that
object. The API response contains the headers, status code, response
body and JSON.
@bkrausz
Copy link
Contributor

bkrausz commented Oct 21, 2015

Nice, lgtm. Was considering suggesting getFoo style accessor methods instead of public vars, but I have no strong preference. Other suggestion is to thread ApiResponse further up to _requestRaw, but again, no preference.

kyleconroy added a commit that referenced this pull request Oct 22, 2015
@kyleconroy
Merge pull request #206 from stripe/last-response
579e7cb 
Save the last response made on StripeObjects
@kyleconroy kyleconroy merged commit 579e7cb into master Oct 22, 2015
@brandur brandur mentioned this pull request Jan 14, 2016
Merged
@brandur brandur mentioned this pull request Jan 23, 2016
Closed
@brandur brandur deleted the last-response branch January 23, 2016 01:04
@ob-stripe ob-stripe mentioned this pull request Nov 14, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kyleconroy @brandur @bkrausz @kjc-stripe