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

Expose exception message via #to_s #90

Closed
wants to merge 2 commits into from

Conversation

leohemsted
Copy link
Contributor

Recreating @tijmenb's PR (#89) to get concourse to run it.

tijmenb and others added 2 commits January 6, 2020 20:37
Exceptions in Ruby return the exception message when calling `#to_s` on
it:

```
> irb
irb(main):001:0> StandardError.new("Hi").to_s
=> "Hi"
```

Because this is default behaviour, libraries sometimes depend on `to_s`
to get a readable version of the exception message. For example, Sentry
does it to the exception a name:

https://github.com/getsentry/raven-ruby/blob/065cf973cb0ad3672d0d213e80b
6e8a9a8e6f8cb/lib/raven/event.rb#L144

Currently, Sentry reports do not show the error message set when
raising the exception - only “Notifications::Client::BadRequestError”.

This is because we’re only defining `RequestError#message`. This is
apparently not the same as initialising an exception with a message, as
you can see here:

```
> be rails c
Loading development environment (Rails 6.0.2.1)
[1] pry(main)>
Notifications::Client::BadRequestError.new(OpenStruct.new(code: "hey",
body: {errors: [{error: "BadRequestError", message: "Can’t send to this
recipient using a team-only API key"}] }.to_json )).to_s
=> "Notifications::Client::BadRequestError"
[2] pry(main)>
Notifications::Client::BadRequestError.new(OpenStruct.new(code: "hey",
body: {errors: [{error: "BadRequestError", message: "Can’t send to this
recipient using a team-only API key"}] }.to_json )).message
=> "BadRequestError: Can’t send to this recipient using a team-only API
key"
```

The issue was introduced in
#73 /
#72, which
removed the explicit `to_s` method.

This commit fixes the issue by constructing the message . Note that
we’ve renamed the `message` method, but calling `e.message` still works
because it’s part of the exception interface. Another option would be
to add the `to_s` method back (like I experimented with in
https://github.com/DFE-Digital/apply-for-postgraduate-teacher-training/p
ull/1024), but this solution seems the most Ruby-esque and relies on
less knowledge of Exception internals.

This change is backwards compatible.
@leohemsted leohemsted closed this Jan 7, 2020
@leohemsted leohemsted deleted the tijmenb-exception-messages branch January 7, 2020 17:11
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.

None yet

2 participants