Prevent Plug.ErrorHandler from re-raising error after callback

Questions / Help
, ,
1

Background

I have a simple Plug router with a PUT endpoint that receives a JSON body. To parse it I am using Plug.Parsers.

Problem

The Plug.Parsers plug works fine and puts the json inside conn.body_params. However, if the JSON I am receiving is malformated, my application explodes with errors. To prevent this I am using Plug.ErrorHandler but since it re-raises the error after, the app still explodes.

Code

This is my router.

defmodule Api do
  use Plug.{Router, ErrorHandler}

  alias Api.Controllers.{Products, NotFound}

  plug Plug.Logger
  plug :match
  plug Plug.Parsers,
    parsers: [:urlencoded, :json],
    pass:  ["text/*"],
    json_decoder: Jason
  plug :dispatch

  put "/products",    do: Products.process(conn)

  match _, do: NotFound.process(conn)

  def handle_errors(conn, %{kind: _kind, reason: _reason, stack: _stack}) do
    send_resp(conn, conn.status, "Something went wrong")
  end
end

It should be noted that in reality Products.process is not (or should not be) called because Plug.Parsers raises before.

And this is my test:

    test "returns 400 when the request is not a valid JSON" do
      # Arrange
      body_params = "" # this is not valid JSON

      conn =
        :put
        |> conn("/products", body_params)
        |> put_req_header("accept", "application/json")
        |> put_req_header("content-type", "application/json")

      # Act
      conn = Api.call(conn, Api.init([]))

      # Assert
      assert conn.state == :sent
      assert conn.status == 400
      assert conn.resp_body == "Invalid JSON in body request"
    end

Error

As you can probably guess, I am expecting the request to return 400 and a nice error message. Instead I get this:

test PUT /products returns 400 when the request is not a valid JSON (ApiTest)
test/api_test.exs:143
** (Plug.Conn.WrapperError) ** (FunctionClauseError) no function clause matching in Api.Controllers.Products.handle_flow_response/2
code: conn = Api.call(conn, @opts)
stacktrace:
(api 0.1.0) lib/api/controllers/products.ex:39: Api.Controllers.Products.handle_flow_response(:ok, %Plug.Conn{adapter: {Plug.Adapters.Test.Conn, :…}, assigns: %{}, before_send: [#Function<1.129014997/1 in Plug.Logger.call/2>], body_params: %{}, cookies: %Plug.Conn.Unfetched{aspect: :cookies}, halted: false, host: “www.example.com”, method: “PUT”, owner: #PID<0.406.0>, params: %{}, path_info: [“cars”], path_params: %{}, port: 80, private: %{plug_route: {“/products”, #Function<1.102964628/2 in Api.do_match/4>}}, query_params: %{}, query_string: “”, remote_ip: {127, 0, 0, 1}, req_cookies: %Plug.Conn.Unfetched{aspect: :cookies}, req_headers: [{“accept”, “application/json”}, {“content-type”, “application/json”}], request_path: “/products”, resp_body: nil, resp_cookies: %{}, resp_headers: [{“cache-control”, “max-age=0, private, must-revalidate”}], scheme: :http, script_name: , secret_key_base: nil, state: :unset, status: nil})
(api 0.1.0) lib/plug/router.ex:284: Api.dispatch/2
(api 0.1.0) lib/api.ex:1: Api.plug_builder_call/2
(api 0.1.0) lib/plug/error_handler.ex:65: Api.call/2
test/api_test.exs:154: (test)

I am rather dumbfounded.

Failed Fix

To avoid this I tried modifying the handle_errors function to the following, but it still failed:

def handle_errors(conn, %{kind: _kind, reason: _reason, stack: _stack}) do
    send_resp(conn, conn.status, "Something went wrong")
    {:error, :something_went_wrong}
end

No matter what I do, the code always flows with “:ok”. Nothing I do seems to have control over the error.

Question

How can I prevent this error from re-raising and simply return the nice error message I have in my test?

1 Like
2

Take a look at the tests for Plug.ErrorHandler for ideas about how to test this:

For your specific situation, the error message shows body_params: %{} - because Plug.Parsers.JSON specifically documents that an empty body will parse to %{}

1 Like
3

Thanks for the heads up regarding the empty map, I really should have seen that.
However, passing an empty string was merely an example of a use case. I should have changed the example to something more complex, like a malformed JSON:

"[{\"id\": 1}"

In this example, the array is not closing, so it produces the following error:

  1. test PUT /cars returns 400 when the request has invalid JSON body (ApiTest)
    test/api_test.exs:157
    ** (Plug.Parsers.ParseError) malformed request, a Jason.DecodeError exception was raised with message “unexpected end of input at position 10”
    code: conn = Api.call(conn, @opts)
    stacktrace:
    (plug 1.10.4) lib/plug/parsers/json.ex:88: Plug.Parsers.JSON.decode/2
    (plug 1.10.4) lib/plug/parsers.ex:313: Plug.Parsers.reduce/8
    (api 0.1.0) lib/api.ex:1: Api.plug_builder_call/2
    (api 0.1.0) lib/plug/error_handler.ex:65: Api.call/2
    test/api_test.exs:168: (test)

Which makes sense, but the app still explodes.

After reading the tests, I don’t see how I can prevent it from exploding the app.
Receiving a malformed JSON is not an exception, is something that will happen somewhat regularly because of the nature of the clients.

I don’t want to blow up every time that happens, just to return a nice error message and move on.

As it stands, I really don’t understand what Plug.HandleErrors is useful for, if the app still blows up anyway.

4

This is one of the places where you’ll need to differentiate between doing an http requests and using plug/phoenix test helpers for creating requests.

Just because your requests blows up here – and it’s supposed to blow up – doesn’t mean your client will receive no response. send_resp/3 in your error handler will make sure the client to the http request is being sent a response. That the process for the web requests blows up afterwards is totally irrelevant. Cowboy will catch that, close the connection and be fine.

For tests you want the error to not be swallowed, so you can catch what’s going wrong. You can also assert_raise if you expect an error. If you want to assert that a client for the http request is not affected you’ll need to do a proper http request using something like httpoison or similar (also make sure to enable the server for the test env, server: true). Basically the misconception here it that conn(conn, "/products", body_params) is doing an http request. It more or less calls MyApp.Endpoint.call(conn, …), even more bare bone for plain plug. No cowboy involved in the slightest. So if you want to actually assert the error handling, which involves more than just the plug parts, you need to fall back to a more involved testing method.