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

original_exception and backtrace are not present even if passed in error! #2470

Closed
ericproulx opened this issue Jul 14, 2024 · 1 comment · Fixed by #2471
Closed

original_exception and backtrace are not present even if passed in error! #2470

ericproulx opened this issue Jul 14, 2024 · 1 comment · Fixed by #2471

Comments

@ericproulx
Copy link
Contributor

ericproulx commented Jul 14, 2024

Since #2468, error! signature has been aligned across different places. Unfortunately, even if you pass a backtrace and/or original_exception arguments, these latter won't be part of the returned message even if backtrace: true and/or original_exception: true are set at the rescue_from level.

Here are the specs

 # json
context 'rescue_from include backtrace and original exception' do
  subject { JSON.parse(last_response.body) }

  let(:app) do
    Class.new(Grape::API) do
      format :json

      rescue_from :all, backtrace: true, original_exception: true do |e|
        error!(e.message, 500, nil, e.backtrace, e)
      end

      params do
        requires :param, type: Integer
      end

      get '/' do
        'it works'
      end
    end
  end

  before do
    get '/'
  end

  it { is_expected.to include('error' => a_kind_of(String), 'backtrace' => a_kind_of(Array), 'original_exception' => a_kind_of(String)) }
end

#xml
context 'rescue_from include backtrace and original exception' do
  subject { Grape::Xml.parse(last_response.body)['error'] }

  let(:app) do
    Class.new(Grape::API) do
      format :xml

      rescue_from :all, backtrace: true, original_exception: true do |e|
        error!(e.message, 500, nil, e.backtrace, e)
      end

      params do
        requires :param, type: Integer
      end

      get '/' do
        'it works'
      end
    end
  end

  before do
    get '/'
  end

  it { is_expected.to have_key('backtrace') & have_key('original-exception') }
end

#txt
context 'rescue_from include backtrace and original exception' do
  subject { last_response.body }

  let(:app) do
    Class.new(Grape::API) do
      format :txt

      rescue_from :all, backtrace: true, original_exception: true do |e|
        error!(e.message, 500, nil, e.backtrace, e)
      end

      params do
        requires :param, type: Integer
      end

      get '/' do
        'it works'
      end
    end
  end

  before do
    get '/'
  end

  it { is_expected.to include('backtrace', 'original exception') }
end

Although if we specify backtrace: true and/or original_exception: true and pass the original exception as message like

rescue_from :all, original_exception: true, backtrace: true do |e|
  error!(e, 500, nil, e.backtrace, e)
end

It will break with an exception when format is json

# exception
# NoMethodError:
#    undefined method `merge' for an instance of Grape::Exceptions::ValidationErrors
#  ./lib/grape/error_formatter/json.rb:13:in `call'
#  ./lib/grape/middleware/error.rb:49:in `format_message'

 context 'rescue_from include backtrace and original exception' do
  subject { JSON.parse(last_response.body) }

  let(:app) do
    Class.new(Grape::API) do
      format :json

      rescue_from :all, original_exception: true, backtrace: true do |e|
        error!(e, 500, nil, e.backtrace, e)
      end

      params do
        requires :param, type: Integer
      end

      get '/' do
        'it works'
      end
    end
  end

  before do
    get '/'
  end

  it { is_expected.to include('error' => a_kind_of(String), 'backtrace' => a_kind_of(Array), 'original_exception' => a_kind_of(String)) }
end
@numbata
Copy link
Contributor

numbata commented Jul 15, 2024

Ouch. Will look into.

numbata added a commit to numbata/grape that referenced this issue Jul 15, 2024
ericproulx pushed a commit that referenced this issue Jul 15, 2024
…error! (#2471)

* Fixes #2470 Expose original_exception and/or backtrace if present

* Update CHANGELOG.md
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 a pull request may close this issue.

2 participants