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

Mutations with payloads defined in the new post 1.8 DSL cannot handle lists of errors. #2566

Closed
stanishev opened this issue Oct 28, 2019 · 1 comment

Comments

@stanishev
Copy link
Contributor

Given the following mutation:

class BaseMutation < GraphQL::Schema::Mutation
end

module Mutations
  class UpdateSomething < BaseMutation
    description "Updates Something"
    payload_type Types::Output::User

    def resolve
      # something happens that causes multiple errors to be returned. 
      [GraphQL::ExecutionError.new("Error!")]
    end
  end
end

and the following output type defined using the new class based DSL (post 1.8):

module Types
  class BaseOutputObject < GraphQL::Schema::Object
    field_class Types::SeField
  end
end

module Types::Output
  class User < Types::BaseOutputObject
    graphql_name "User"

    field :email, String, null: false
    field :name, String, null: true
 end
end

and running the following query

mutation {  update_something() { email } }

Produces the following error:

{
  "errors": [
    {
      "message": "RuntimeError:         Failed to implement User.email, tried:\n\n        - `Types::Output::User#email`, which did not exist\n        - `Array#email`, which did not exist\n        - Looking up hash key `:email` or `\"email\"` on `[#<GraphQL::ExecutionError: Error!>]`, but it wasn't a Hash\n\n        To implement this field, define one of the methods above (and check for typos)\n",
      "type": "runtime"
    }
  ]
}

THIS IS THE ISSUE ^^^

It seems that the GraphQL error results are being treated by the gem as if they're a valid result.

However, using the old pre 1.8 DSL for the output type above:

module Types::Output
  User = GraphQL::ObjectType.define do
    name "User"

    field :email, !types.String
    field :name, types.String
  end
end

Produces the expected result:

{
  "data": null,
  "errors": [
    {
      "message": "Error!",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "update_something"
      ]
    }
  ]
}

This seems to be related to what's happening in this line in the ProxiedResolve class in member/instrumentation.rb:

if ctx.skip == result || ctx.schema.lazy?(result) || result.nil? || result.is_a?(GraphQL::ExecutionError) || ctx.wrapped_object

It's not entirely clear to me what instrumentation is and why the above doesn't affect types and fields defined using the pre 1.8 DSL, but I'll link a PR with a fix that seems to address this.

@rmosolgo
Copy link
Owner

Duplicate of #1587, please see this comment a workaround:

#1587 (comment)

I don't plan to work on it anymore because it doesn't seem required by the spec. In fact, the spec seems to assume that a field with either return a value or raise an error; there's no mention of fields returning multiple errors.

However, if you want to implement it, I'll accept your PR!

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

2 participants