Grape: Regression?

[dblock]

See https://github.com/ruby-grape/grape/pull/1953#issuecomment-572064622

[dnesteryuk]

Unfortunately, it isn't easy to fix. I would recommend to drop my commit from the master and do

git push origin -f

or I can open another PR where I will restore the ReverseStackableValues. :disappointed:

[dm1try]

or I can open another PR where I will restore the ReverseStackableValues. 😞

👍, an explicit revert is more suitable for the master

[dblock]

Yeah, someone please PR a revert.

@dnesteryuk Care to explain the problem?

[dm1try]

closed via #1961

[dnesteryuk]

When rescue handlers get added, every handler is added to a separate hash.

[{ RuntimeError => handler }, { StandardError => handler }]

Even though they are defined in one endpoint

rescue_from RuntimeError do
 # catch
end

rescue_from StandardError do
 # catch
end

So, after reverting this array, we got the wrong order. If handlers defined in an endpoint were in one hash

[{ RuntimeError => handler, StandardError => handler }]

after adding inherited values it would look like this:

[
  { RuntimeError => inheritedHandler }, 
  { RuntimeError => handler, StandardError => handler }
]

my solution would've worked. However, the stackable values doesn't work this way. Actually, any structure doesn't work this way.

I tried a few other ideas, but they didn't work because of namespaces and copies of settings, there are lots of things happening with settings. Grape really shuffles them a lot.

I am sorry guys for troubles :disappointed:

[dblock]

I am sorry guys for troubles 😞

No trouble at all. Bug was caught before release. A test case was added (thanks @dmitry!) that ensures rescue handlers happen in predictable order, which was clearly a miss and potentially a serious regression in the future because of some other change. I call this a win!