Add response_metadata to Web API errors #311
Conversation
| @@ -10,6 +10,18 @@ def initialize(message, response = nil) | |||
| super message | |||
| @response = response | |||
| end | |||
|
|
|||
| def error | |||
There was a problem hiding this comment.
This is sort of a horrible name. I used it because it's the name of the field in the response body, but I'm open to changing it. One alternative is error_code (and error_codes below).
| @@ -10,7 +10,7 @@ class Client | |||
|
|
|||
| def initialize(options = {}) | |||
| Slack::Web::Config::ATTRIBUTES.each do |key| | |||
| send("#{key}=", options[key] || Slack::Web.config.send(key)) | |||
| send("#{key}=", options.fetch(key, Slack::Web.config.send(key))) | |||
There was a problem hiding this comment.
This turned out to not be directly relevant, but I noticed that if there was a truthy value in the config that it could not be overridden with a falsey value.
|
To be honest, I dislike this. We are introducing a global flag to control formatting of an error message. It just feels wrong to me. Can we split this PR into two parts and get the non-controversial parts merged? I like |
|
Sure, I see what you're saying. I'll pull out the global option and the corresponding change to the error message. |
|
@dblock updated |
jmanian
changed the title
| @@ -4,13 +4,22 @@ | |||
| RSpec.describe Slack::Web::Api::Errors::SlackError do | |||
| let(:client) { Slack::Web::Client.new } | |||
|
|
|||
| it 'provides access to the response object', vcr: { cassette_name: 'web/auth_test_error' } do | |||
There was a problem hiding this comment.
I would also leave that old test in-place and add checks for its .message, .error, .response_metadata, it's a simpler case, but will prevent breaking it in the future.
|
LGTM, feel free to (re)add that missing test and squash-merge on 💚 |
|
Needs |
|
Oh I just saw your comment. I resolved by following the rule. |
Relates to #310
SlackError#errorandSlackError#response_metadatamethods give direct access to these fields in Slack's response body.Original description below, from the first draft of this PR that did a lot more:
There are two main things happening here:
SlackError#errorandSlackError#response_metadatamethods give direct access to these fields in Slack's response body.verbose_errorsoption changes themessageonSlackErrorto includeresponse_metadata.I hit one snag, which is that I wasn't sure how to access the client instance within
Slack::Web::Faraday::Response::RaiseError, which is where the error message is currently assembled. This means that I could not read theverbose_errorssetting off of the client itself, but instead had to pull it from the config, which means that it is not configurable at the client instance level.