Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Handle errors from HTTP requests #22

Open
jdfrens opened this issue Jun 9, 2021 · 0 comments
Open

Handle errors from HTTP requests #22

jdfrens opened this issue Jun 9, 2021 · 0 comments

Comments

@jdfrens
Copy link
Contributor

jdfrens commented Jun 9, 2021

I and another developer have had problems testing Airbrake.report/2 today. Unfortunately, Airbrake.report/2 always returns :ok with no output or logging, even if the HTTPoison.post/3 fails.

The situation

We make the HTTP POST in Airbrake.Worker.send_report/3 (a private function):

@http_adapter.post(notify_url(), json_encoder.encode!(payload), @request_headers)

The value returned here is never used or pattern-matched against.

I filled in the necessary values and ran the equivalent command from the iex console, and I got {:error, %HTTPoison.Error{id: nil, reason: :timeout}} as a response sometimes; other times I would get a 200 OK success.

I tried setting timeout options: [timeout: 50_000, recv_timeout: 50_000]. Now the requests always go through, but I'm getting a lot of 500s (with a giant "Internal Server" HTML page as the body). So ultimately it seems that there is a problem on the Airbrake.io side of things.

So an internal server error from Aibrake.io is bad, but not under our control. This library should react to a problem like this better that ignoring it.

Options

Right now, airbrake_client swallows these errors completely. This is not ideal; it would be nice if there were some record of the problem.

Some options we might like to consider:

  1. Bump up the timeouts.
    • By default they are 8s to make a connection, 5s to wait for a response. Of course, the longer the timeout, the more it could become a bottleneck.
    • This only solves timeout errors. An internal server error from Airbrake.io isn't handled any better with just this solution.
  2. Log the error.
    • This could be tricky because we don't want Airbrake.LoggerBackend to log the error since it would sent a report to Airbrake.io. If Airbrake.io were to go down completely, we'd end up with an infinite loop between Airbrake.Worker and Airbrake.LoggerBackend.
    • Airbrake.LoggerBackend uses IO.inspect/1 when it has an exception to report:
      exception -> IO.inspect(exception)
      .
  3. Retry the POST.
    • Try the call three times in the code, maybe with greater timeouts each time (especially with a timeout error).
    • Send the request back to the Airbrake.Worker process.
    • Queue up requests to send later.

1 and 2 have simple solutions that could be effective. All of the solutions to 3 raises the complexity of the library significantly, and when a solution breaks, it'll break even harder than what we have now (thrashing processes and supervisors).

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

No branches or pull requests

1 participant