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

Add examples of alternative HTTP clients in the documentation #539

Closed
ream88 opened this issue Mar 30, 2023 · 5 comments
Closed

Add examples of alternative HTTP clients in the documentation #539

ream88 opened this issue Mar 30, 2023 · 5 comments

Comments

@ream88
Copy link

ream88 commented Mar 30, 2023

Not sure if we should add it directly to the code base, maybe mentioning in the README.md is enough. 😊

This is an alternative implementation of Sentry.HTTPClient based on finch, instead of hackney. req, finch and mint are kinda the new kids on the block, and I tend to use them more often nowadays and having multiple HTTP libraries in one code base is somewhat a code smell imo:

# config/config.exs
config :sentry, client: Sentry.FinchClient

# lib/sentry/finch_client.ex
defmodule Sentry.FinchClient do
  @behaviour Sentry.HTTPClient

  def child_spec() do
    Supervisor.child_spec({Finch, name: __MODULE__}, id: __MODULE__)
  end

  def post(url, headers, body) do
    request = Finch.build(:post, url, headers, body)

    case Finch.request(request, __MODULE__) do
      {:ok, %Finch.Response{status: status, headers: headers, body: body}} ->
        {:ok, status, headers, body}

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

EDIT: Updated the example, thx for the help @arcanemachine!

@arcanemachine
Copy link

arcanemachine commented May 25, 2023

For anyone else who is new to Sentry, you can configure Sentry to use this client by adding the following to your config/config.exs:

config :sentry,
  client: Sentry.FinchClient

Thanks @ream88!

EDIT: I'm still pretty new to Elixir/Phoenix, so I'm not sure if adding it to the config.exs (which I understand is a compile-time configuration) is the best. However, I believe that this way should work fine since the configuration does not change after compiling the project (e.g. in a Mix release). Happy to hear suggestions on the matter if I am wrong.

@arcanemachine
Copy link

arcanemachine commented May 25, 2023

@ream88 Everything seems to working fine, but I'm getting a Dialyzer error in the child_spec() function with your Finch client implementation:

The inferred return type of child_spec/0 
         ({'Elixir.Finch', [{'name', 'Elixir.Sentry.FinchClient'}, ...]}) has nothing in common with 
          {_,
           {atom(), atom(), 'undefined' | [any()]},
           'permanent' | 'temporary' | 'transient',
           'brutal_kill' | 'infinity' | non_neg_integer(),
           'supervisor' | 'worker',
           'dynamic' | [atom()]} |
          #{'id' := _,
            'start' := {atom(), atom(), 'undefined' | [any()]},
            'modules' => 'dynamic' | [atom()],
            'restart' => 'permanent' | 'temporary' | 'transient',
            'shutdown' => 'brutal_kill' | 'infinity' | non_neg_integer(),
            'significant' => boolean(),
            'type' => 'supervisor' | 'worker'}, which is the expected return type for the callback of the 'Elixir.Sentry.HTTPClient' behaviour (ElixirLS Dialyzer)

I am about to go on a deep dive of some interesting Elixir features, but right now I'm not familiar with supervisors, implementing behaviors, child_spec(), etc... Do you know if your implementation of the child_spec() is correct, or do you think it something that needs some refinement in the future?

Thanks

@ream88
Copy link
Author

ream88 commented May 25, 2023

Does this fix the problem?

def child_spec() do
  Supervisor.child_spec({Finch, name: __MODULE__}, id: __MODULE__)
end

@arcanemachine
Copy link

Well, it made the Dialyzer error go away, and I hope that prevents any issues that could arise (I'm still learning). Thanks a lot!

@ruslandoga
Copy link
Contributor

ruslandoga commented Jun 13, 2023

👋 @ream88 @arcanemachine

Note that there are some issues with that kind of Finch client implementations right now: #514

@whatyouhide whatyouhide changed the title Include alternatives HTTP clients Add examples of alternative HTTP clients in the documentation Jul 12, 2023
@whatyouhide whatyouhide added this to the 9.0 milestone Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants