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

Added Request-ID to API calls. #191

Closed
wants to merge 3 commits into from
Closed

Added Request-ID to API calls. #191

wants to merge 3 commits into from

Conversation

joshluongo
Copy link

I needed to get access to the Request-ID that is sent from the API for a project I was working on.

In order to make this work I have made two small changes.

  • The Request-ID is stored in the RequestOptions as $requestId.
  • In order to access $requestId I have added added basic check to the magic method in StripeObject.

You can access it by getting _requestid from any stripe object.

I hope this comes in handy for someone!

joshluongo and others added 3 commits August 28, 2015 10:36
Whenever stripe responds with a `Request-ID` it is stored in the
request options. This allows for easy access to it after a request is
made.
Made it so you can call `_requestid` on stripe objects to get the
request id.
@bkrausz
Copy link
Contributor

bkrausz commented Aug 30, 2015

I agree that we should have a way to get response headers, but I don't think storing it on RequestOptions is the right way to expose it: they are meant to be sent per-request, and are fairly independent from the response. For example, they get glued through to future requests (ex: calling retrieve and then save uses the same RequestOptions by default, though I believe it does make a new options object).

I think I'd rather expose a getLastResponseHeaders accessor on each object, that is overridden on each refresh. Then we wouldn't need the _ key magic, and it's relatively clear that each refresh will update it. It also covers all headers, so things like Stripe-Version would be fetchable as well.

The other option would be a static method on ApiRequestor that does the same thing, but static methods are less appealing.

@kyleconroy - any thoughts on this?

@joshluongo
Copy link
Author

@bkrausz I agree with you, My method is not the right way to achieve this, I needed the functionality asap for my implementation so it met my requirements.

I think having getLastResponseHeaders would be a worthwhile change to make.

@kyleconroy
Copy link
Contributor

@bkrausz I think we should probably make this more generic by adding a getResponse method that returns the actually API response, status code, body and all.

@kyleconroy
Copy link
Contributor

See the work done in #206

@kyleconroy kyleconroy closed this Oct 20, 2015
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