Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

endpoint_run.grape callback is called without middlewares #2010

Open
Kukunin opened this issue Mar 12, 2020 · 1 comment · May be fixed by #2021
Open

endpoint_run.grape callback is called without middlewares #2010

Kukunin opened this issue Mar 12, 2020 · 1 comment · May be fixed by #2021
Labels

Comments

@Kukunin
Copy link

Kukunin commented Mar 12, 2020

Hello. I'm trying to add the monitoring by subscribing to 'endpoint_run.grape' via ActiveSupport::Notifications.subscribe.

It works well until an exception occurs inside API. I have a bunch of rescue_from blocks to return different status codes depending on exceptions, such as

rescue_from ActiveRecord::RecordNotFound do
  error!({ error: :not_found }, 404)
end

The problem is when I call an endpoint which raises ActiveRecord::RecordNotFound, it really returns 404 status code, but in my callback data[:endpoint].status is still 200 (however data[:exception_object] is not null).

A quick look at the code shows, that callback wraps only the main call, without middlewares. I believe, it should wrap middlewares too because:

  • middlewares are part of the business logic. It's not good to have 404 returned to a client and 200 reported to the monitoring.
  • timing will be more accurate because the middlewares can eat time too.
@dblock
Copy link
Member

dblock commented Mar 13, 2020

This does look like a bug, or something that maybe we can address in a backward compatible way, appreciate a PR.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants