ruby-grape / grape

An opinionated framework for creating REST-like APIs in Ruby.
http://www.ruby-grape.org
MIT License
9.88k stars 1.22k forks source link

Error when rescue Grape::Exceptions::ValidationErrors in json format with backtrance and original_exception #2472

Closed ericproulx closed 1 month ago

ericproulx commented 1 month ago

Following #2471, I found that this case is raising an error.

describe 'rescue Grape::Exceptions::ValidationErrors in json format with backtrace and original_exception' do
  let(:app) do
    Class.new(Grape::API) do
      format :json

      rescue_from Grape::Exceptions::ValidationErrors, backtrace: true, original_exception: true do |e|
        error!(e, 418, {}, e.backtrace, e)
      end

      params do
        requires :beer
      end

      get '/' do
      end
    end
  end

  before { get '/' }

  context 'with json response type format' do
    subject { last_response.status }

    it { is_expected.to eq(418) }
  end
end

Our README shows the same kind of example but without original_exception and backtrace.

Nonetheless, It works for XML and TXT because of the simple ternary condition while JSON is doing something different when Grape::Exceptions::ValidationErrors

If we want to add original_exception and backtrace I think we'll need to enhance Grape::Exceptions::ValidationErrors with a new function to include its when dumping JSON.

ericproulx commented 1 month ago

@numbata if you want to take a look

ericproulx commented 1 month ago

Unfortunately, since Grape::Exceptions::ValidationErrors generates an array when formatted in JSON, we won't be able to add the properties at the root level. Also, adding them to each element of the array might be to much.

numbata commented 1 month ago

Sure, would be happy to take it for tomorrow, but it feels as if you are already deep into it.

numbata commented 1 month ago

Unfortunately, since Grape::Exceptions::ValidationErrors generates an array when formatted in JSON, we won't be able to add the properties at the root level. Also, adding them to each element of the array might be to much.

Agreed! And what if we could just skip the backtrace and the original exception filling if the message structure is not suitable for it? Something like #2480