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

Clarification Question. Errors #23

Open
morgz opened this issue Feb 23, 2021 · 3 comments
Open

Clarification Question. Errors #23

morgz opened this issue Feb 23, 2021 · 3 comments

Comments

@morgz
Copy link

morgz commented Feb 23, 2021

Thanks for the great library, I've got a point of confusion I'd like help understanding.

The primary philosophy is that messages generated by invalid and/or unexpected user input are DATA, and should be returned as such. On the other hand, errors made in using an API - like querying a field that doesn’t exist -, are actually ERRORS and should be returned as errors.

are actually ERRORS and should be returned as errors.

This line confused me. Typically, my GraphQL responses have "data" and "errors" at the root. If, before passing into the build_payload function my resolution already has errors, I'm surprised they are being manipulated into the payload object and not being left as errors. That's what I though it meant by ERRORS should be returned as errors.

On the client side this gives me a bit of a problem. For example:

  • I check auth before resolving, it fails, I add an {:error, :unauthorised}
  • I look for this error on the client to refresh tokens etc.
  • On my mutations which use build_payload, these errors will get moved to messages.
  • This leaves me with 2 different checks.

My ideal would be a change like this to the library, but please feel free to tell me why it'd a bad idea!

def build_payload(%{errors: _errors} = resolution, _config), do: resolution
@morgz morgz changed the title Clarification Question Clarification Question. Errors Feb 23, 2021
@1sarah
Copy link

1sarah commented Jul 20, 2022

Hi, Do you have a fork of this librarry that fixes this issue. We are experiencing the same issue and would not like to break the graphql standards.

@bforchhammer
Copy link

Not the OP, but we use the following helper currently:

  defp build_better_payload(%{errors: errors} = resolution, config) do
    case errors do
      # No errors -> build payload (successful=true with result)
      [] -> build_payload(resolution, config)
      # Changeset errors -> build payload (successful=false with error messages)
      [%Ecto.Changeset{}] -> build_payload(resolution, config)
      # Other errors -> keep as is, ie. render as top level "errors" in response
      _ -> resolution
    end
  end

(Used as middleware &build_better_payload/2 on the respective fields).

@TheArrowsmith
Copy link

TheArrowsmith commented Apr 24, 2024

I had a similar problem. I'm authenticating users with this middleware:

defmodule MyApp.Schema.Middleware.Authenticate do
  @behaviour Absinthe.Middleware

  alias MyApp.Accounts.User

  def call(resolution, _) do
    case resolution.context do
      %{current_user: %User{}} ->
        resolution

      _ ->
        Absinthe.Resolution.put_result(resolution, {:error, :unauthorized})
    end
  end
end

But if authorization fails, I want this to be an error response, not a data response.

However, this is the only homegrown :error that I don't want to AbsintheErrorPayload to handle in its normal way. In other places I'm returning error tuples from my resolvers e.g. {:error, :not_found} and I do want AEP to convert this into a regular payload with succesful: false etc. (Not saying this is better or worse, it's just what I've settled on for my own app.)

So my solution is to do something similar to @bforchhammer, except simpler because I don't need to handle the generic case:

  defp build_payload(%{errors: errors} = resolution, config) do
    if :unauthorized in errors do
      resolution
    else
      AbsintheErrorPayload.Payload.build_payload(resolution, config)
    end
  end

That's in my Schema module.

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

4 participants