Skip to content

feat: Client that uses Req. #946

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+24 −0
Open

feat: Client that uses Req. #946

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions lib/sentry/req_client.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
defmodule Sentry.ReqClient do
@behaviour Sentry.HTTPClient

@moduledoc """
HTTP client for Sentry based on Req.

This client implements the `Sentry.HTTPClient` behaviour.
It's based on the [Req](https://github.com/wojtekmach/req) Elixir HTTP client,
which is an *optional dependency* of this library. If you wish to use another
HTTP client, you'll have to implement your own `Sentry.HTTPClient`. See the
documentation for `Sentry.HTTPClient` for more information.
"""

@impl true
def post(url, headers, body) do
case Req.post(url, headers: headers, body: body) do
{:ok, %{status: status, headers: headers, body: body}} ->
{:ok, status, headers, body}
Comment on lines +16 to +18
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential bug: The success case for Req.post incorrectly pattern matches on a plain map (%{}) instead of the returned %Req.Response{} struct, which will cause a MatchError.

  • Description: The case statement in Sentry.ReqClient incorrectly pattern matches on a plain map, %{status: status, ...}, for the success case of a Req.post call. According to the Req library's documentation, successful calls return a struct, %Req.Response{...}. In Elixir, a struct will not match a plain map pattern. This mismatch will cause a MatchError to be raised for every successful HTTP request sent via this client, rendering it non-functional for its primary purpose of sending events to Sentry.

  • Suggested fix: Update the pattern match in the case statement to expect a %Req.Response{} struct. Specifically, change the line {:ok, %{status: status, headers: headers, body: body}} to {:ok, %Req.Response{status: status, headers: headers, body: body}}.
    severity: 0.9, confidence: 1.0

Did we get this right? 👍 / 👎 to inform future reviews.


{:error, error} ->
{:error, error}
end
end
end