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

feat: Include details for web api errors #483

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kevinrobayna
Copy link

fixes: #482

Currently when we get the error message we get no details of what went wrong unless we get the response_metadata for the error which is not intuitive as most people when they get an error and print the exception they would expect to see as much details as possible.

Additionally updated ruby-version in the project to be the latest as most people would be using that one as their default, though not changing the one required by the gem

fixes: slack-ruby#482

Currently when we get the error message we get no details of what went
wrong unless we get the response_metadata for the error which is not
intuitive as most people when they get an error and print the exception
they would expect to see as much details as possible.

Additionally updated ruby-version in the project to be the latest as
most people would be using that one as their default, though not
changing the one required by the gem
@@ -22,6 +22,12 @@ def errors
def response_metadata
response.body.response_metadata
end

def to_s
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the approach I took here is to put as many details as possible but without empty details i.e. the errors list would not be added if there are none, same as the response metadata.

@@ -1 +1 @@
2.7.6
3.2.0
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me know if you want me to remove this from the PR 😅

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, but this is just the default version being used, it has little consequence on anything. We should probably just delete this file.

Copy link
Collaborator

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some specs with actual text? Let's see how it looks?

If we're going to start returning complex detailed errors we should adopt a better structured approach and even support localization. Check out https://github.com/dblock/ruby-enum/blob/master/lib/ruby-enum/errors/base.rb and how it constructs errors, I would copy everything from there.

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.

Error details aren't passed through
2 participants