graphql icon indicating copy to clipboard operation
graphql copied to clipboard

Cypher Directive returning Interface list causes Server Error `Expected Iterable`

Open jnterry opened this issue 3 years ago • 6 comments

Describe the bug

A @cypher directive which returns a list of interfaces causes a graphql server error: Expected Iterable

Type definitions

interface Media {
	id      : ID! @id
	title   : String!
	likedBy : [Person!] @relationship(type: "LIKES", direction: IN)
	similar : [Media!] @cypher(statement: """
		MATCH (this)<-[:LIKES]-(:Person)-[:LIKES]->(other:Media)
		RETURN other
	""")
}

type Book implements Media @node(additionalLabels: ["Media"]) {
	id      : ID!
	title   : String!
	likedBy : [Person!]
	similar : [Media!]

	pageCount: Int!
}

type Film implements Media @node(additionalLabels: ["Media"]) {
	id      : ID!
	title   : String!
	likedBy : [Person!]
	similar : [Media!]

	runTime: Int!
}

type Person {
	name   : String
	likes  : [Media!] @relationship(type: "LIKES", direction: OUT)
}

To Reproduce

Seed the database as follows:

createPeople(input: [{
    name: "Bob",
    likes: { create: [
        { node: {Book: { title: "Harry Potter",      pageCount: 300, }}},
        { node: {Book: { title: "Lord of the Rings", pageCount: 400, }}}
    ]}
}]) {
    info { nodesCreated }
}

Now execute the following query:

query {
  books {
    title
    similar { title }
  }
}

The following error is returned by the server:

{
    "message": "Expected Iterable, but did not find one for field \"Book.similar\".",
    "locations": [{ "line": 4,   "column": 5 }],
    "path": ["books", 0, "similar"]
}

Expected behavior

Query should resolve successfully.

Note that a simplified schema without the use of an interface - but using the same @cypher query, and using the additonal Media label behaves as expected:

type Book @node(additionalLabels: ["Media"]) {
	id      : ID! @id
	title   : String!
	likedBy : [Person!] @relationship(type: "LIKES", direction: IN)
	similar : [Book!] @cypher(statement: """
		MATCH (this)<-[:LIKES]-(:Person)-[:LIKES]->(other:Media)
		RETURN other
	""")

	pageCount: Int!
}

type Person {
    name   : String
    likes  : [Book!] @relationship(type: "LIKES", direction: OUT)
}

Running the same graphql query works as expected:

{
  "data": {
    "books": [
      { "title": "Harry Potter",      "similar": [{ "title": "Lord of the Rings" }] },
      { "title": "Lord of the Rings", "similar": [{ "title": "Harry Potter" } ]}
    ]
  }
}

This would strongly suggest the issue is not the use of the additionalLabel on @node, but rather is just an issue of returning an interface from cypher.

Attempted Work Around

To force return an iterable, I updates the cypher query to:

MATCH (this)<-[:LIKES]-(:Person)-[:LIKES]->(other:Media)
RETURN COLLECT(other)

This produces a new error:

Abstract type \"Media\" must resolve to an Object type at runtime for field \"Book.similar\". Either the \"Media\" type should provide a \"resolveType\" function or each possible type should provide an \"isTypeOf\" function..

This may itself be indicative of a bug in @neo4j/graphql -> it should be able to work out the real type of the node as it is fully aware of the primary label vs additonalLabels due to the @node directive.

We can help @neo4j/graphql further by changing the @cypher query to:

MATCH (this)<-[:LIKES]-(:Person)-[:LIKES]->(other:Media)
RETURN COLLECT(other { .*, __resolveType: apoc.coll.subtract(labels(other), ['Meda'])[0] })

This now "works" in that the server returns the expected data for the above query - however, if the selection set on the similar field is updated to include nested relationships (eg: similar { likedBy { id } }) the server complains again - presumably since it can't query a subfield on the returned object - it relies on cypher returning Node instances to support this.

System (please complete the following information):

Additional context

The presence of the Media label on the nodes implementing the interface is required by my schema - since we also use it to enforce a unique constraint on id across all Media nodes. @unique on Book and Film does not prevent a Book sharing an id with a Film.

jnterry avatar Jan 28 '22 09:01 jnterry

Many thanks for raising this bug report @jnterry. :bug: We will now attempt to reproduce the bug based on the steps you have provided.

Please ensure that you've provided the necessary information for a minimal reproduction, including but not limited to:

  • Type definitions
  • Resolvers
  • Query and/or Mutation (or multiple) needed to reproduce

If you have a support agreement with Neo4j, please link this GitHub issue to a new or existing Zendesk ticket.

Thanks again! :pray:

neo4j-team-graphql avatar Jan 28 '22 09:01 neo4j-team-graphql

We've been able to confirm this bug using the steps to reproduce that you provided - many thanks @jnterry! :pray: We will now prioritise the bug and address it appropriately.

neo4j-team-graphql avatar Feb 01 '22 09:02 neo4j-team-graphql

I'm coming up against this issue as well - is there a workaround if it is low priority? Thanks

pandemosth avatar Mar 22 '22 05:03 pandemosth

@pandemosth, basically the "attempted workaround" in the original issue here. You need to collect() the results before returning them.

The "fix" for this is more of a feature, hence why this has been classed as low priority: #903

We tend to prioritise any custom Cypher issues lower than others, because almost without fail, there will be a workaround within the custom Cypher itself. Because of the nature of the custom Cypher directive, we will be constantly chasing our tails if we keep on trying to cater for edge cases.

darrellwarde avatar Mar 22 '22 11:03 darrellwarde

@darrellwarde - Its worth noting however that the "attempted workaround" has a massive drawback - you can no longer query nested objects (IE: traversing relationships to new nodes) which is an unworkable solution for many use cases. I think this is because the custom cypher is forced to return objects, rather than actual Node instances to avoid server errors produced by the @neo4j/graphql JS code

I do not know of any "workaround" purely in the custom cypher that adequately resolves this issue - I think it requires support from the library.

This is proving to be a blocker in my own transition from the old neo4j-graphql-js library

jnterry avatar Mar 22 '22 11:03 jnterry

A lot of this is conceptually what we expect from Cypher results - if a GraphQL field is returned as a list, then we expect a list to be returned from the Cypher, not each member of the list in a different record.

And yes, the automatic projection of relationships won't work in every scenario (such as this workaround), because honestly there's just a whole bunch of assumptions that we make about the Cypher in order to do that.

We appreciate the feedback, it will be considered when looking at our "low priority" bugs!!

darrellwarde avatar Mar 22 '22 14:03 darrellwarde

Just validated that this now works as expected.

darrellwarde avatar Feb 21 '24 11:02 darrellwarde