craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.28k stars 635 forks source link

[4.x]: Can't alias GraphQL entries field to "author" (internal server error) #14057

Closed tremby closed 10 months ago

tremby commented 10 months ago

What happened?

Description

I'm getting an internal server error when I attempt to alias an entries field to the identifier author.

Steps to reproduce

  1. Make a new section called authors. Fields don't matter.
  2. Make another new section; mine is called recipes
  3. In its default entry type, add an entries field (mine is called authorEntry) so the content author can choose an authors entry.
  4. Make an authors entry and a recipes entry (mine has slug the-last-word), in which the author is selected.
  5. Attempt to run the following GQL query:
    MyQuery {
      recipesEntries(slug: "the-last-word") {
        ... on recipes_default_Entry {
          author: authorEntry {
            title
          }
        }
      }
    }

Expected behavior

Getting the author entry's title back, looking something like

recipesEntries: [
  { author: [{ title: "xyz" }] }
]

Actual behavior

{
  "errors": [
    {
      "debugMessage": "Cannot assign craft\\elements\\Entry to property craft\\elements\\Entry::$_author of type craft\\elements\\User|false|null",
      "message": "Internal server error",
      "extensions": {
        "category": "internal"
      },
      "file": "/my-path/vendor/craftcms/cms/src/elements/Entry.php",
      "line": 1576,
      "trace": [
        {
          "file": "/my-path/vendor/craftcms/cms/src/services/Elements.php",
          "line": 3061,
          "call": "craft\\elements\\Entry::setEagerLoadedElements('author', array(1))"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/services/Elements.php",
          "line": 2874,
          "call": "craft\\services\\Elements::_eagerLoadElementsInternal('craft\\elements\\Entry', array(1), array(1))"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/elements/db/ElementQuery.php",
          "line": 3111,
          "call": "craft\\services\\Elements::eagerLoadElements('craft\\elements\\Entry', array(1), array(1))"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/elements/db/ElementQuery.php",
          "line": 1542,
          "call": "craft\\elements\\db\\ElementQuery::_createElements(array(1))"
        },
        {
          "file": "/my-path/vendor/yiisoft/yii2/db/Query.php",
          "line": 251,
          "call": "craft\\elements\\db\\ElementQuery::populate(array(1))"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/db/Query.php",
          "line": 247,
          "call": "yii\\db\\Query::all(null)"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/elements/db/ElementQuery.php",
          "line": 1581,
          "call": "craft\\db\\Query::all(null)"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/gql/base/ElementResolver.php",
          "line": 51,
          "call": "craft\\elements\\db\\ElementQuery::all()"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/gql/queries/Entry.php",
          "line": 110,
          "call": "craft\\gql\\base\\ElementResolver::resolve(null, array(2), array(2), instance of GraphQL\\Type\\Definition\\ResolveInfo)"
        },
        {
          "file": "/my-path/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 623,
          "call": "craft\\gql\\queries\\Entry::craft\\gql\\queries\\{closure}(null, array(2), array(2), instance of GraphQL\\Type\\Definition\\ResolveInfo)"
        },
        {
          "file": "/my-path/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 549,
          "call": "GraphQL\\Executor\\ReferenceExecutor::resolveFieldValueOrError(instance of GraphQL\\Type\\Definition\\FieldDefinition, instance of GraphQL\\Language\\AST\\FieldNode, instance of Closure, null, instance of GraphQL\\Type\\Definition\\ResolveInfo)"
        },
        {
          "file": "/my-path/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 1195,
          "call": "GraphQL\\Executor\\ReferenceExecutor::resolveField(GraphQLType: Query, null, instance of ArrayObject(1), array(1))"
        },
        {
          "file": "/my-path/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 264,
          "call": "GraphQL\\Executor\\ReferenceExecutor::executeFields(GraphQLType: Query, null, array(0), instance of ArrayObject(1))"
        },
        {
          "file": "/my-path/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 215,
          "call": "GraphQL\\Executor\\ReferenceExecutor::executeOperation(instance of GraphQL\\Language\\AST\\OperationDefinitionNode, null)"
        },
        {
          "file": "/my-path/vendor/webonyx/graphql-php/src/Executor/Executor.php",
          "line": 156,
          "call": "GraphQL\\Executor\\ReferenceExecutor::doExecute()"
        },
        {
          "file": "/my-path/vendor/webonyx/graphql-php/src/GraphQL.php",
          "line": 161,
          "call": "GraphQL\\Executor\\Executor::promiseToExecute(instance of GraphQL\\Executor\\Promise\\Adapter\\SyncPromiseAdapter, instance of GraphQL\\Type\\Schema, instance of GraphQL\\Language\\AST\\DocumentNode, null, array(2), null, 'MyQuery', null)"
        },
        {
          "file": "/my-path/vendor/webonyx/graphql-php/src/GraphQL.php",
          "line": 93,
          "call": "GraphQL\\GraphQL::promiseToExecute(instance of GraphQL\\Executor\\Promise\\Adapter\\SyncPromiseAdapter, instance of GraphQL\\Type\\Schema, 'query MyQuery {\n  recipesEntries(slug: \"the-last-word\") {\n    ... on recipes_default_Entry {\n      author: authorEntry {\n        title\n      }\n    }\n  }\n}\n', null, array(2), null, 'MyQuery', null, array(26))"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/services/Gql.php",
          "line": 506,
          "call": "GraphQL\\GraphQL::executeQuery(instance of GraphQL\\Type\\Schema, 'query MyQuery {\n  recipesEntries(slug: \"the-last-word\") {\n    ... on recipes_default_Entry {\n      author: authorEntry {\n        title\n      }\n    }\n  }\n}\n', null, array(2), null, 'MyQuery', null, array(26))"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/controllers/GraphqlController.php",
          "line": 195,
          "call": "craft\\services\\Gql::executeQuery(instance of craft\\models\\GqlSchema, 'query MyQuery {\n  recipesEntries(slug: \"the-last-word\") {\n    ... on recipes_default_Entry {\n      author: authorEntry {\n        title\n      }\n    }\n  }\n}\n', null, 'MyQuery', true)"
        },
        {
          "call": "craft\\controllers\\GraphqlController::actionApi()"
        },
        {
          "file": "/my-path/vendor/yiisoft/yii2/base/InlineAction.php",
          "line": 57,
          "function": "call_user_func_array(array(2), array(0))"
        },
        {
          "file": "/my-path/vendor/yiisoft/yii2/base/Controller.php",
          "line": 178,
          "call": "yii\\base\\InlineAction::runWithParams(array(2))"
        },
        {
          "file": "/my-path/vendor/yiisoft/yii2/base/Module.php",
          "line": 552,
          "call": "yii\\base\\Controller::runAction('api', array(2))"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/web/Application.php",
          "line": 305,
          "call": "yii\\base\\Module::runAction('graphql/api', array(2))"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/web/Application.php",
          "line": 606,
          "call": "craft\\web\\Application::runAction('graphql/api', array(2))"
        },
        {
          "file": "/my-path/vendor/craftcms/cms/src/web/Application.php",
          "line": 284,
          "call": "craft\\web\\Application::_processActionRequest(instance of craft\\web\\Request)"
        },
        {
          "file": "/my-path/vendor/yiisoft/yii2/base/Application.php",
          "line": 384,
          "call": "craft\\web\\Application::handleRequest(instance of craft\\web\\Request)"
        },
        {
          "file": "/my-path/web/index.php",
          "line": 12,
          "call": "yii\\base\\Application::run()"
        }
      ]
    }
  ],
  "data": {
    "recipesEntries": null
  }
}

Notes:

Also:

Craft CMS version

4.5.11.1

PHP version

8.2.7

Operating system and version

Not sure exactly other than that it's linux, but it's running in a docker container based on php:8.2-fpm

Database type and version

Postgres 14

Image driver and version

Imagick

Installed plugins and versions

Amazon S3   2.0.3
Control Panel CSS   2.6.0
Embedded Assets     3.2.0
Formie  2.0.43
Navigation  2.0.22
Neo     3.9.10
Queue Heartbeat     1.1.2
Redactor    3.0.4
Remote Backup   4.1.3
Retour  4.1.14
Sentry Logger   4.1.4
SEOmatic    4.0.36
Typed link field    2.1.5
tremby commented 10 months ago

When searching for similar issues I came across this comment from Brandon:

[other bug] is happening because photo and a few other fields have special cases in the eager loading logic (e.g. entries’ author fields), so we need to identify all of those and avoid using eager-loading aliases in those cases, from the GraphQL side.

Seems relevant.

brandonkelly commented 10 months ago

Entries already have author properties. So at best, maybe the error could be improved so that it clarifies the conflict. But I don’t think it should be possible to override existing properties with aliases.

tremby commented 10 months ago

But I don’t think it should be possible to override existing properties with aliases.

Aliases don't override anything. I don't see why there should be any conflict here as far as Craft is concerned. If I wanted the native author field in addition I could ask for author: authorEntry { title } craftAuthor: author { username } or similar. This is all allowed by the GQL spec so I don't expect an error from Craft.

brandonkelly commented 10 months ago

The problem is that Craft automatically eager-loads any relation fields that are queried in GraphQL, using the same system used to eager-load native properties like author. Currently element classes aren’t able to differentiate between a core eager-loading handle and an alias, so all craft\elements\Entry::setEagerLoadedElements() sees is that author is getting eager-loaded, and doesn’t know author is just a custom alias that was being used for an authorEntry custom field. So it assumes that the passed-in element“s” are the entry’s author.

I was able to fix this for Craft 5, by passing the actual eager-load plan to setEagerLoadedElements(), so elements can access the original eager-loading handle.

That’s a breaking change though, so there’s no way we can fix it for Craft 4.x.

tremby commented 10 months ago

OK, thanks.

So would there also be an issue if someone aliased the Craft author field to something else? Would craftAuthor: author { username } not eager-load?

brandonkelly commented 10 months ago

Correct, because this condition would fail:

https://github.com/craftcms/cms/blob/ed7939dd427b4696d5b6a62e78f4b731c73382b4/src/elements/Entry.php#L619

That will also be resolved by the Craft 5 change, though.

brandonkelly commented 10 months ago

Craft 5.0.0-alpha.4 is out with that fix.