Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Prevent Goliath::Rack::Params from swallowing downstream errors

I ran into this behavior when I wanted to add an error tracking service to my
Goliath application (for me, [Airbrake](https://airbrake.io/), but the specific
service is tangential). To implement this, I added a middleware at the very top
of the chain that delegated downstream while rescuing any exceptions. Roughly:

```ruby
class ExceptionHandlingMiddleware
  include Goliath::Rack::AsyncMiddleware

  def call(env)
    super
  rescue Exception => e
    # track the exception, do some cleanup, return a generic response, etc.
  end
end
```

When I added this middleware to my API, though, it wasn't working. After enough
experimentation, I discovered that it had to do with my use of
Goliath::Rack::Params at some other point in the middleware chain. More than
that, the ordering was what decided if it worked.

The following ordering did not work, even though it's the logical placement for
an exception handling middleware:

```ruby
class API < Goliath::API
  use ExceptionHandlingMiddleware
  use Goliath::Rack::Params
  use MiddlewareThatWasRaisingAnException

  # ...
end
```

I would hit an endpoint, get back a nicely-wrapped HTTP 500 error with the
message raised by MiddlewareThatWasRaisingAnException, but the
ExceptionHandlingMiddleware wasn't doing any of the exception handling.

This ordering *did* work, though:

```ruby
class API < Goliath::API
  use Goliath::Rack::Params
  use ExceptionHandlingMiddleware
  use MiddlewareThatWasRaisingAnException

  # ...
end
```

I would hit an endpoint, get back the HTTP 500 error with the right message,
and the ExceptionHandlingMiddleware was doing its processing.

As it turns out, the culprit was Goliath::Rack::Params itself. Previously,
Goliath::Rack::Params would delegate down the chain with an @app.call(env), but
wrap *that* call in a Goliath::Rack::Validator.safely block. Therefore, any
exceptions raised downstream would get rescued, and the error response
generated by Goliath::Rack::Validator.safely was ultimately returned as the API
response - no exceptions raised for the ExceptionHandlingMiddleware to handle.

But really, the only errors that the Goliath::Rack::Params middleware should be
rescuing have to do with parsing the params. So, quite simply, this commit
moves the @app.call(env) out of the Goliath::Rack::Validator.safely block. If
there was an error in parsing the params, we avoid delegating to @app and
instead just return the validation error response. If there is an exception
raised by @app.call(env), upstream middleware will see it.

Ultimately, if an exception raised by the @app.call(env) in
Goliath::Rack::Params is not handled by upstream middleware (like in my
proposed use case), we're still safe because the likes of Goliath::API#call and
Goliath::Request#process ensure that any unhandled errors will get rescued and
result in some sort of Rack response.
latest commit 34bd7e5597
@ajvondrak ajvondrak authored
Something went wrong with that request. Please try again.