elixir-maru / maru

Elixir RESTful Framework
https://maru.readme.io
BSD 3-Clause "New" or "Revised" License
1.32k stars 85 forks source link

Expanding `rescue_from` #87

Closed jalcine closed 7 years ago

jalcine commented 7 years ago

I've currently done the following to catch exceptions in my current project:

  rescue_from :all, as: error do
    trace = System.stacktrace()

    Logger.error fn -> 
      Sentry.capture_exception(error, [stacktrace: trace, result: :async])
      inspect(error)
    end

    code = case error do
      %Plug.CSRFProtection.InvalidCSRFTokenError{} ->
        :bad_request
      _ ->
        :server_error
    end

    message = cond Map.has_key?(error, "message") do
      is_nil(_) -> nil
      is_boolean(_) -> nil
      _ -> msg
    end

    Explode.with(conn, code, (case message do
      nil -> error
      _ -> message
    end))
  end

Though not elegant, it gets the job done with sending information to the right places and making sure I can get useful debugging data. However, I was looking at Plug.ErrorHandler and was considering a registration system of sorts.

This would reduce the code that looks like the above into something like:

rescue_from :all, with: ExceptionHandlers.CatchAll
rescue_from [:NotFoundError, :MatchError], with: ExceptionHandlers.NotFound

These underlying implementations could share logic that handles things like statsd reporting, Sentry, logging, etc.

The signature for the block in rescue_from can be reused here, provided we use a protocol like the following:


defprotocol Maru.ErrorHandler do
  @doc "Parses the provided error data."
  def handle_error(conn, error)
end
falood commented 7 years ago

for now, maru support handle error like this

  rescue_from [MatchError, RuntimeError], with: :custom_error

  defp custom_error(conn, exception) do
    conn
    |> put_status(500)
    |> text(exception.message)
  end

maybe what we need to do is just make with accept a list, like

  rescue_from [MatchError, RuntimeError], with: [:custom_error1, :custom_error2]

and the logic could be conn |> custom_error1(exception) |> custom_error2(exception)

do you think it's enough for you? I think it's more easier than a new Maru.ErrorHandler protocol 🙂

jalcine commented 7 years ago

Hm, I was not aware of this second format. That actually handles what I need well. I used that do something like

rescue_from :all, with: :catch_all_errors

defp catch_all_errors(conn, exception) do
   App.Maru.ErrorHandler.handle_error(conn, exception)
end

I'm in favor of having the separate module because it encapsulates everything about error handling so that I can call it from elsewhere in the application. This works amazingly though. Thank you!