-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
added spec for expected headers when error raised #1723
base: master
Are you sure you want to change the base?
Conversation
Here's an example of a CHANGELOG.md entry: * [#1723](https://github.com/ruby-grape/grape/pull/1723): Added spec for expected headers when error raised - [@gencer](https://github.com/gencer). Generated by 🚫 danger |
When we set headers using `before`, `after` or just before `error!` raised, we expect them to transferred with error response together. * First spec will be `success` due to there is no error. * Second test will fail at the moment because when an error raised, all headers will be ignored. They shouldn't be. added spec for expected headers when error raised When we set headers using `before`, `after` or just before `error!` raised, we expect them to transferred with error response together. * First spec will be `success` due to there is no error. * Second test will fail at the moment because when an error raised, all headers will be ignored. They shouldn't be.
I've amended and squashed due to incorrect file format according to grape's rubocop rules. I've fixed it by this merge. |
You should break up the specs: are headers not added from before and after in the |
I will break up the specs asap. For the question; I will back with more proper specs as soon as possible. |
I think we shouldn't change this behavior to avoid regressions for all existing users. PR documentation that says that the headers will not be included on error and specs that expect the opposite behavior of the ones here. Lets find a way to create the opposite behavior. The |
Let's say we did not changed this behavior. As you mentioned we have But fact is, for specific reasons, this could be enough. But I need to show some headers to clients even if they got One thing that i can suggest is making error-specific before/after statements like;
|
I like |
Definitely. This will allow us to specify headers, actions and anything else on every error, rather then define them on each |
When we set headers using
before
,after
or just beforeerror!
raised, we expect them to transferred with error response together.success
due to there is no error.See: #1719