Skip to content

Conversation

@wowinter13
Copy link

See: #1975

@grape-bot
Copy link

grape-bot commented Jan 21, 2020

1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Generated by 🚫 danger

@dblock
Copy link
Member

dblock commented Jan 21, 2020

Doesn't look like it's failing?

@wowinter13 wowinter13 force-pushed the fix/rescue_from_specs branch from 1880e88 to c55a9c7 Compare January 21, 2020 17:26
@wowinter13
Copy link
Author

wowinter13 commented Jan 21, 2020

Sorry, a bit of a misunderstanding
UPD: fixed

@wowinter13 wowinter13 requested a review from dblock January 21, 2020 19:06
@dblock
Copy link
Member

dblock commented Jan 21, 2020

Cool, now that we have a spec all we need is a fix! Give it a shot.

@wowinter13
Copy link
Author

@dblock, I have some ideas on how to fix it, but want to discuss due to lack of knowledge.

Somehow we need to use rescue_handler_for_any_class before any other handler, BUT only in case of nested rescue blocks. For now I don't see an obvious solution without stacking all errors (like :rescue_all, :rescue_grape_exceptions etc) to some namespace. Maybe there is any better realization like using reverse_stacking from descendants to the ancestor? Or..?

@dblock
Copy link
Member

dblock commented Jan 22, 2020

Feels like maybe the tree of rescue handlers should be traversed as a tree?

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

Labels

None yet

3 participants