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

Improve error message if error response has resonse_metadata #201

Closed

Conversation

vbanthia-zz
Copy link

@vbanthia-zz vbanthia-zz commented Jan 30, 2018

Some slack errors responses such as dialog.open also contains response_metadata field with more useful information related to error. This PR will improve error message by showing that in client error message.

Before

Slack::Web::Api::Errors::SlackError - validation_errors

After

2018-01-29 17:51:56 - Slack::Web::Api::Errors::SlackError - validation_errors Response Metadata: [ERROR] Element 0 field `label` cannot be longer than 24 characters:

https://api.slack.com/methods/dialog.open

@vbanthia-zz vbanthia-zz mentioned this pull request Jan 30, 2018
@dangerpr-bot
Copy link

2 Warnings
⚠️ There’re library changes, but not tests. That’s OK as long as you’re refactoring existing code.
⚠️ Unless you’re refactoring existing code, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#201](https://github.com/slack-ruby/slack-ruby-client/pull/201): Improve error message if error response has resonse_metadata - [@vbanthia](https://github.com/vbanthia).

Generated by 🚫 danger

@dblock
Copy link
Collaborator

dblock commented Jan 30, 2018

I think this shows that we did a minimum job in the response handler here earlier :) Let's add a response_metadata field (and possibly others) to SlackError, assign the response itself to the class as a field and expose anything useful. Then that class should implement to_s and combine all the errors there.

Everything needs tests, too.

@vbanthia-zz
Copy link
Author

Will do over weekend.

@dblock
Copy link
Collaborator

dblock commented May 7, 2018

Bump @vbanthia ?

@henrik
Copy link

henrik commented Mar 22, 2019

@vbanthia, just a friendly ping :) I know how life can get in the way.

@dblock
Copy link
Collaborator

dblock commented Mar 22, 2019

@henrik maybe you want to continue this PR?

@henrik
Copy link

henrik commented Mar 22, 2019

@dblock I was considering it! Just ended my lab day, but I might do some more Slack work next time around, and if so I'll see :) If anyone else wants to do it, please feel free.

@dblock
Copy link
Collaborator

dblock commented Apr 8, 2019

I'll close this. Feel free to reopen if traction.

@dblock dblock closed this Apr 8, 2019
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.

5 participants