Make Web API response headers accessible

[jmanian]

The response headers in Slack's Web API contain information that is sometimes useful. For example x-oauth-scopes is a list of scopes that the token has. This is the only way (that I know of) to check the scopes on an existing token through the API. My use case is I'd like to be able to use auth.test to check the scopes on a token via x-oauth-scopes.

The current implementation doesn't give a way to access the response headers on a successful request, since it returns only the body: https://github.com/slack-ruby/slack-ruby-client/blob/ebf98319cf9d89ad4e75dbca0ae8ecf94a855aa3/lib/slack/web/faraday/request.rb#L36

The headers are accessible on the error object raised on failed requests since the entire response object is on the error:

slack_error.response.headers

I'm trying to think how we could make the headers accessible on successful responses. Some bad ideas:

1. Shove them into the body object under the key response_headers. This object is then wrapped in Slack::Messages::Message and returned. I don't like this because it pollutes the body.
2. An option that can be passed to any request (possibly also set on a client) that causes it to return something different:
   1. The entire raw response object, giving access to the body as well as the headers (and much else).
   2. [body, response_headers]
   3. etc.

[jmanian]

@dblock any ideas?

[dblock]

3. Return response in https://github.com/slack-ruby/slack-ruby-client/blob/ebf98319cf9d89ad4e75dbca0ae8ecf94a855aa3/lib/slack/web/faraday/request.rb#L36, which is private, and change all methods above to do, for example, request(:get, path, options).body. Make request public and invoke it to get headers.
4. Return response in https://github.com/slack-ruby/slack-ruby-client/blob/ebf98319cf9d89ad4e75dbca0ae8ecf94a855aa3/lib/slack/web/faraday/request.rb#L36, and change all template code to call .body, so def get(path, options = {}) now returns a response object and not a .body. It's an API change, but YOLO.
5. Extract invoke from request, so the latter becomes invoke(method, path, options).body. Make both public. Maybe we should then rename request to body, as well, and alias it to request with deprecation.

[jmanian]

I see what you mean on #379 about option (2)(i) being bad from an API standpoint. Options 3-5 are less convenient, but since this is likely a rare use case that's OK with me. In my case I need to do this in only 1 spot at the moment.

I guess my leaning is option 5.

[dblock]

Want to try to implement 5?

[dblock]

Thinking about this more, maybe we should (also) add client.oauth_scopes that invokes the auth test api and returns the result from the header. Another alternative would be to supply an optional response middleware that always inspects the response for oauth_scopes and stores/updates that.

[jmanian]

The middleware is an interesting idea. Are you thinking it would store them on the instance of the client?

[dblock]

The middleware is an interesting idea. Are you thinking it would store them on the instance of the client?

Yep.

[jmanian]

I'm taking a crack at the middleware approach now. Do you think it should be optional via a new web config setting/attribute?

[dblock]

I'm taking a crack at the middleware approach now. Do you think it should be optional via a new web config setting/attribute?

Only if it creates significant overhead. Without looking at the code, I would try and enable that by default.