Better Access to Response Headers

[sedouard]

On a successful API request the Stripe Node SDK doesn't provide a natural way of accessing response headers which can be useful in the case you're interested in response metadata (such as Idempotency-Key and Request-ID.

For example today it is possible to do this but it is a little bit awkward since response metadata is exposed through an event emitter interface, while the response body is exposed through a callback interface:

const stripe = require("stripe")(
  "sk_test_XXXXXXXXXXXXXXXXXX"
);
function fetchUser () {
  stripe.on('response', (response) => {
    // response has parsed headers in it
    console.dir(response)
  })
  stripe.customers.retrieve('cus_BhemmnI4d8eFjR',
    function(err, customer) {
      if (err) {
        console.dir(err)
      }
      console.dir(customer)
    }
  );  
}

It would be a much better experience if this data was passed in the callback like so:

const stripe = require("stripe")(
  "sk_test_XXXXXXXXXXXXXXXXXX"
);
function fetchUser () {
  stripe.customers.retrieve('cus_BhemmnI4d8eFjR',
    function(err, customer) {
      if (err) {
        console.dir(err)
      }
      console.dir(customer.response)
    }
  );  
}

My idea is that we add headers, statusCode and known Stripe headers such as idempotencyKey to go along with requestId to StripeResource here

Thoughts?

[remi-stripe]

assigning to @jlomas-stripe who built the feature originally in #366

[brandur-stripe]

assigning to @jlomas-stripe who built the feature originally in #366

Thanks!

And yeah, it seems fine to me to expose idempotencyKey given that we've already got requestId on there. I wonder if we should add something like a headers field onto lastResponse? It's not too likely any of our special fields is going to class with a property in a response object, but mixing them all into one big bucket like we're doing now seems like poor hygiene.

[jlomas-stripe]

@sedouard You can already get the requestID at customer.lastResponse.requestId, but the idempotencyKey isn't available in the same place. That said, it is available in the customer.lastResponse object, but you'll currently have to dig for it.

@brandur it might make sense to expose anything that we allow providing, maybe as:

thingy.lastResponse = {
    requestId: 'req_ABC123XYZ',
    headers: { /* 'extra' headers that were used */ },
}

Or alternatively, we could just put in the things we know will be there, or even just straight up make it match whatever comes out in on('response')...?

[sedouard]

Personally I like the idea of matching what comes out of on('response'). It's tiny and has all the custom headers and important metadata. I can't think of much use cases which would need the other standard HTTP headers

[brandur-stripe]

I think it'd probably be already to add an idempotencyKey helper on lastResponse, but given that it's called lastResponse, we should be aiming to model it like a response as much as possible so that a user familiar with HTTP can easily guess at how to properly use it (e.g. should have data, should have status code, should have headers).

[stevene-stripe]

So on @brandur-stripe's point the current 'esponse (and request) objects coming from the request and response events don't conform to conventional js naming (they're snake_cased):

{
  api_version: 'latest',
  account: 'acct_TEST',       // Only present if provided
  idempotency_key: 'abc123',  // Only present if provided
  method: 'POST',
  path: '/v1/charges',
  status: 402,
  request_id: 'req_Ghc9r26ts73DRf',
  elapsed: 445                // Elapsed time in milliseconds
}

However lastResponse is currently camelCased which is a bit confusing.

I think it would be great to model lastResponse with our added idempotencyKey helper and a shape similar to that of the responses from request with camelCased keys. It won't agree with our emitted response object but maybe its better to go on a more conventional path than to conform to a less conventional existing schema.

[brandur-stripe]

I think it would be great to model lastResponse with our added idempotencyKey helper and a shape similar to that of the responses from request with camelCased keys. It won't agree with our emitted response object but maybe its better to go on a more conventional path than to conform to a less conventional existing schema.

+1.

[jlomas-stripe]

Welp, I fell off the world apparently. ¯\_(ツ)_/¯

@sedouard @brandur-stripe:

If I recall correctly, I modeled these on the parameter names that get provided, rather than camelCasing them as one would normally see in Node.

At this point, I'd say switching up the events' property names would be a breaking change, so I'm not sure if it's something that could/should be done here?

That said, adding idempotencyKey to thingy.lastRequest seems totally reasonable - but I wonder if it should be idempotencyKey or idempotency_key to match with the response event, and I wonder if we should also add request_id (in addition to requestId) for the same reason.

I also wonder if api_version and stripe_account should be included too, since they're also present in the event...?

[jlomas-stripe]

Per magical conversations elsewhere - and to remind myself for when I finally do this - I'm planning on adding the following to lastResponse from the header:

• idempotencyKey;
• stripeAccount;
• apiVersion;

[brandur-stripe]

Per magical conversations elsewhere - and to remind myself for when I finally do this - I'm planning on adding the following to lastResponse from the header:

Sorry about the hugely delayed response here Jonathan, but yep, that sounds good to me.

[polybuildr]

Hello! 👋 I've tried to do this in #952, hope it's still relevant... Let me know if this isn't meant for an external contributor, though - that's totally fine too! :)

[polybuildr]

Resolved in #952!