absinthe-graphql / absinthe_plug

Plug support for Absinthe, the GraphQL toolkit for Elixir
https://hex.pm/packages/absinthe_plug
MIT License
261 stars 163 forks source link

Catch exceptions, raise Plug.Conn.WrapperError #278

Open vincedevendra opened 1 year ago

vincedevendra commented 1 year ago

We have a Phoenix application that expects the [:phoenix, :endpoint, :stop] event to be executed, but I noticed that this was not happening when uncaught exceptions originated from this plug.

This is because Plug.Telemetry registers this event by using Plug.Conn.register_before_send, which, in turn, adds a key to conn.private.

When there is an uncaught exception, Phoenix catches the error to render a standard error response here. When this sends the response, the telemetry callback is run and the event is executed.

Notice that if there is a Plug.Conn.WrapperError, the conn from it is used, but if not, conn is what's in scope at the time the RenderErrors plug runs. This happens to be at the top of the middleware stack before Plug.Telemetry registers the callback. Thus, no [:phoenix, :endpoint, :stop] event is executed.

Note also that bug reporting plugs may also expect request parsers etc. to have surfaced important bits of info on the conn (we ran into this issue as well, but worked around it).

This issue can be solved by rescuing errors in call and using Plug.Conn.Wrapper.reraise/4

vincedevendra commented 1 year ago

I should note that this likely only effects Phoenix applications without a router, since Phoenix Router wraps errors. As our application is solely a GraphQL server, we don't have/need a router. That's in line with the advice given here