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

enhancement: error.message should return a string. #78

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xiaoronglv
Copy link
Contributor

Problem

Sentry is a popular exception monitoring tool for ruby, but it fails to
capture the error raised by checkr ruby gem.

After some investigation, I found the checkr sometime set an array to
the exception message.

Here is an example

exception.message
=> ["First name must only contain letters, numbers, spaces, hyphens, apostrophes, periods, and commas."]

Usually, ruby exception's message method should return a string as an
abstraction of error and developer save the detail information to
internal instance variable.

Solution

This PR is to convert the message variable to a string to avoid crashing
the monitoring tool.

Related PR

Sentry crashes when error message is an array.

getsentry/sentry-ruby#1879

## Problem

Sentry is a popular exception monitoring tool for ruby, but it fails to
capture the error raised by checkr ruby gem.

After some investigation, I found the checkr sometime set an array to
the exception message.

Here is an example

```
exception.message
=> ["First name must only contain letters, numbers, spaces, hyphens, apostrophes, periods, and commas."]
```

Usually, ruby exception's `message` method should return a string as an
abstraction of error and developer save the detail information to
internal instance variable.

## Solution

This PR is to convert the message variable to a string to avoid crashing
the monitoring tool.
@xiaoronglv
Copy link
Contributor Author

Hi @akshay-8d66 ,

could you please help to review this bugfix when you get time?

@st0012
Copy link

st0012 commented Sep 5, 2022

I think the real question is: why does message need to be redefined?

This should also work while preserving the behavior of Exception#message

module Checkr
  class CheckrError < StandardError
    attr_reader :http_status
    attr_reader :http_body
    attr_reader :json_body

    def initialize(message=nil, http_status=nil, http_body=nil, json_body=nil)
      super(message)
      @http_status = http_status
      @http_body = http_body
      @json_body = json_body
    end
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants