absinthe_error_payload icon indicating copy to clipboard operation
absinthe_error_payload copied to clipboard

Clarification Question. Errors

Open morgz opened this issue 4 years ago • 3 comments

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 avatar Feb 23 '21 09:02 morgz

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.

1sarah avatar Jul 20 '22 14:07 1sarah

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).

bforchhammer avatar Jul 20 '22 14:07 bforchhammer

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.

TheArrowsmith avatar Apr 24 '24 09:04 TheArrowsmith