graphql-go icon indicating copy to clipboard operation
graphql-go copied to clipboard

How to return an error only at one element of an array?

Open marcosgmeli opened this issue 5 years ago • 5 comments

Hi all, you have made a great work on the repo, thanks for it.

I am new at GraphQL but for what I understand from the specs if an element of an array can't be returned it should create an error, but unless the wrapped element is a nonNull, it should not null the entire list, what makes sense thinking on a best effort approach, and is a similar result to when a NonNull field of an element fails.

I think that is similar to be able to treat every element of an array as a field.

We have tried several ways but can't find the one to achieve this behavior, is it posible? the specs really support it?

Thanks in advance.

marcosgmeli avatar Oct 15 '20 20:10 marcosgmeli

I'm not sure I follow. Could you, please, give an example?

pavelnikolov avatar Mar 04 '21 07:03 pavelnikolov

I'll jump in here because I'm currently facing the exact same problem. Let's say you get a list of ids returned by some aggregation, but this list is not secured via foreign key constraints for some reason. When you now want to resolve this list of ids into their corresponding entities, but it fails for one of these, there is little you can currently do to report this issue in the response. So either you just drop the failing item silently or you report an error in the resolver, but then all the items being resolved successfully are lost as well.

type entityRepository struct{}

func (e *entityRepository) Lookup(id int) (*Entity, error) {
	// imagine some db lookup here
	return nil, nil
}

type Entity struct {
	Id int
	Name string
	Deleted bool
}

type resolver struct {}

func (r resolver) Entities() (list []Entity, err error) {
	// imagine this to be the raw list of ids coming from some aggregation, but not secured via fk constraint 
	var ids = []int{1337, 4711, 90210}

	er := new(entityRepository)
	for _, id := range ids {
		entity, err := er.Lookup(id)
		if err != nil {
			// what to do here???
			// option 1: we could just raise the error, but then all the items resolved successfully will be lost
			// option 2: we could just drop/log the issue, but then the user will never know
			continue
		}
		list = append(list, *entity)
	}
	return
}

I personally would wish for some sort of soft error here. So it will be reported in the error field in the response, but the successful cases are still present in the response.

sGy1980de avatar Oct 06 '21 16:10 sGy1980de

@marcosgmeli, @sGy1980de I verified and this is a bug indeed. Can be reproduced by:

package main

import (
        "log"
        "net/http"

        graphql "github.com/graph-gophers/graphql-go"
        "github.com/graph-gophers/graphql-go/relay"
)

type query struct{}

func (*query) Words() ([]*string, error) {
	word1 := "word1"
	word3 := "word3"
	word4 := "word4"
	res := []*string{&word1, nil, &word3, &word4, nil}
	return res, fmt.Errorf("something went wrong with indices 1 and 4")
}

func main() {
	s := `
		type Query {
			words: [String]!
		}
	`
	schema := graphql.MustParseSchema(s, &query{})
	http.Handle("/query", &relay.Handler{Schema: schema})
	log.Fatal(http.ListenAndServe(":8080", nil))
}

and then running:

curl -XPOST -d '{"query": "{ words }"}' localhost:8080/query

we get back:

{
  "errors": [
    {
      "message": "something went wrong with indices 1 and 4",
      "path": [
        "words"
      ]
    }
  ],
  "data": null
}

Where I would expect to receive:

{
  "errors": [
    {
      "message": "something went wrong with indices 1 and 4",
      "path": [
        "words"
      ]
    }
  ],
  "data": {
    "words": ["word1", null, "word3", "word4", null]
  }
}

pavelnikolov avatar Mar 02 '23 15:03 pavelnikolov

Actually, after more carefully reading the spec, the library works perfectly fine where in the above example if we don't return the error, the result would be:

{
  "data": {
    "words": [
      "word1",
      null,
      "word3",
      "word4",
      null
    ]
  }
}

which corresponds to option 2) in @sGy1980de 's comment. This exactly what the spec recommends that we should do:

Screenshot 2023-03-02 at 17 37 58

I am closing this issue as the library already supports the result coercion recommended in the spec.

pavelnikolov avatar Mar 02 '23 15:03 pavelnikolov

The only thing that I am not sure about is that some fields return an Error: <error_here> and others have (With logged error). I wonder what does this mean? Maybe it's worth checking the reference implementation of the graphql-js library.

pavelnikolov avatar Mar 02 '23 15:03 pavelnikolov