absinthe-graphql / absinthe

The GraphQL toolkit for Elixir
http://absinthe-graphql.org
Other
4.29k stars 530 forks source link

non_null(list_of(non_null(...))) doesn't propagate list of nil upwards #1071

Open atomkirk opened 3 years ago

atomkirk commented 3 years ago

Environment

Expected behavior

    field :users_who_can_sign, non_null(list_of(non_null(:user))) do
      resolve(fn %{id: field_id}, _, _ ->
        # …this returned [nil, nil, nil]
      end)
    end

It should have propogated null up to the next highest nullable field

Actual behavior

the resulting payload was:

usersWhoCanSign: null

So the spec says, as I'm sure you know:

If a List type wraps a Non-Null type, and one of the elements of that list resolves to null, then the entire list must resolve to null. If the List type is also wrapped in a Non-Null, the field error continues to propagate upwards.

So, it works correctly if it the field wasn't a non_null, but since its non_null, it should have propagated upward

benwilson512 commented 3 years ago

Thanks! So I've sort of reproduced this in https://github.com/absinthe-graphql/absinthe/pull/1072, but if you look at the test failure the data field is actually doing the right thing already today. It's missing the error message, which is bad, but I'm not reproducing the behavior you're seeing.

Can you create a reproduceable example?

ksmithut commented 1 year ago

Here is a minimal example where I was able to reproduce the issue:

`server.ex` ```elixir Application.put_env(:sample, Sample.Endpoint, http: [ip: {127, 0, 0, 1}, port: 4000], server: true ) Mix.install([ {:plug_cowboy, "2.6.1"}, {:jason, "1.4.0"}, {:phoenix, "1.7.2"}, {:absinthe, "1.7.1"}, {:absinthe_plug, "1.5.8"}, {:req, "0.3.6"} ]) defmodule Sample.Schema do use Absinthe.Schema object :todo do field(:id, non_null(:id)) end object :wrapper do # If this field were on the query, I couldn't get the issue to happen field :todos, non_null(list_of(:todo)) do resolve(fn _, _ -> # without the async wrapper, I couldn't get the issue to happen async(fn -> {:error, "error"} end) end) end end query do field :wrapper, :wrapper do resolve(fn _, _ -> {:ok, %{}} end) end end end defmodule Sample.Endpoint do use Phoenix.Endpoint, otp_app: :sample plug(Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: Jason) plug(Absinthe.Plug, schema: Sample.Schema) end {:ok, _} = Supervisor.start_link([Sample.Endpoint], strategy: :one_for_one) # Uncomment the below line and remove the Req.post! call and match to just run the server so you can hit it with curl or whatever. # Process.sleep(:infinity) query = """ query { wrapper { todos { id } } } """ body = %{query: query} |> Jason.encode!() headers = [{"Content-Type", "application/json"}] res = Req.post!("http://localhost:4000/", body: body, headers: headers) # This should be the response %{"data" => %{"wrapper" => nil}} = res.body # This is the response we're getting, but todos is not nil # %{"data" => %{"wrapper" => %{"todos" => nil}}} ```

Run elixir server.ex and it will spin up the server and make the http request and try to match against the expected result.

I went through all sorts of schema permutations, and I believe the issue is when your resolver matches the following criteria:

The above example has a schema more or less like this:

type Todo {
  id: ID!
}

type Wrapper {
  todos: [Todo]!
}

type Query {
  wrapper: Wrapper
}

And this query is made:

query {
  wrapper {
    todos {
      id
    }
  }
}

When the wrapper > todos resolver returns an error asynchronously, then I would expect the response to look something like this:

{
  "data": {
    "wrapper": null
  },
  "errors": [{}]
}

But instead we get:

{
  "data": {
    "wrapper": {
      "todos": null
    }
  },
  "errors": [{}]
}