The Wayback Machine - https://web.archive.org/web/20210106151505/https://github.com/ruby-grape/grape/pull/1982
Skip to content

/ grape

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

Jump to bottom

Added failing spec for nested rescue_from declarations #1982

Open
wants to merge 1 commit into
base: master
from
Open

Added failing spec for nested rescue_from declarations #1982

wants to merge 1 commit into from

Conversation

@wowinter13
Copy link

@wowinter13 wowinter13 commented Jan 21, 2020

See: #1975
@grape-bot
Copy link

@grape-bot grape-bot commented Jan 21, 2020 •
edited

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 dblock commented Jan 21, 2020

Doesn't look like it's failing?
@wowinter13
Added failing spec for nested rescue_from declarations
Loading status checks…
c55a9c7
@wowinter13 wowinter13 force-pushed the wowinter13:fix/rescue_from_specs branch from 1880e88 to c55a9c7 Jan 21, 2020
@wowinter13
Copy link
Author

@wowinter13 wowinter13 commented Jan 21, 2020 •
edited

Sorry, a bit of a misunderstanding
UPD: fixed
@wowinter13 wowinter13 requested a review from dblock Jan 21, 2020
@dblock
Copy link
Member

@dblock 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

@wowinter13 wowinter13 commented Jan 22, 2020

@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 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
Reviewers

@dblock dblock

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@wowinter13 @grape-bot @dblock
You can’t perform that action at this time.