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

Error struct #54

Closed
keathley opened this issue May 9, 2020 · 8 comments
Closed

Error struct #54

keathley opened this issue May 9, 2020 · 8 comments
Assignees

Comments

@keathley
Copy link
Collaborator

keathley commented May 9, 2020

We should have an exception that we can use to capture different errors. I'm not sure if we should wrap over the Mint errors or not. But it seems like we might want to for consistency.

@QuinnWilton
Copy link
Contributor

Reraising Mint.HTTPError into a Finch.Error is probably prudent. I think that the way Mint documents all of the possible error types makes for a very easy to use API, and I think Finch could benefit from taking the same approach: https://github.com/elixir-mint/mint/blob/master/lib/mint/http_error.ex#L56

Do you think that it's worth differentiating between server errors and client errors? I've historically found a lot of value in doing so when working with HTTP clients, to make it easier to determine whether a given error is transient and should be retried, or if it's resulting from bad user input / programmer error.

@whatyouhide
Copy link
Contributor

We don't do the client/server distinction in Mint, so I'm afraid it would be a bit annoying to do here 😄

@keathley keathley self-assigned this Jun 9, 2020
@keathley
Copy link
Collaborator Author

keathley commented Jun 9, 2020

I've started a branch for this. I just need to ensure that we're capturing all of the mint errors correctly

@josevalim
Copy link
Contributor

One option here is to say that we return Exception.t. Then we can return Mint.Error for low-level stuff and Finch.Error for high level stuff. It may be more straight-forward than handling all of the wrapping.

@keathley
Copy link
Collaborator Author

Yeah, that's probably the right way to go. I was worried that having that inconsistency would make it harder for people to pattern match on.

@ijunaid8989
Copy link

is it possible that in this place

children = [
  {Finch,
   name: MyConfiguredFinch,
   pools: %{
     :default => [size: 10],
     "https://hex.pm" => [size: 32, count: 8]
   }}
]

we can also define the return behaviour of success response, such

there is an opportunity to name finch to be your own representation.

MyConfiguredFinch, is it possible to make response your own representation as well and when a request is made.

%Finch.Response{} becomes %MyConfiguredFinch{}

@sneako
Copy link
Owner

sneako commented Jun 20, 2021

is it possible that in this place

children = [
  {Finch,
   name: MyConfiguredFinch,
   pools: %{
     :default => [size: 10],
     "https://hex.pm" => [size: 32, count: 8]
   }}
]

we can also define the return behaviour of success response, such

there is an opportunity to name finch to be your own representation.

MyConfiguredFinch, is it possible to make response your own representation as well and when a request is made.

%Finch.Response{} becomes %MyConfiguredFinch{}

I think this would be better handled within a module in your application. For example, you could define something like this:

defmodule MyApp.MyConfiguredFinch do
  defstruct [...fields]

  def request(request) do
    # or use a `case` if you also want to transform errors
    with {:ok, response} <- Finch.request(request, MyConfiguredFinch) do
      to_custom_response_struct(response)
    end
  end

  defp to_custom_response_struct(response) do
     %__MODULE__{...}
  end
end

@keathley
Copy link
Collaborator Author

Closing this for now.

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

6 participants