craftcms / cms

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

Attempting a GraphQL mutation via the GraphQL API in Craft 3.7.14 throws internal server error if there is a disabled site #9948

Closed humandevs closed 3 years ago

humandevs commented 3 years ago

Description

This is a followup to https://github.com/craftcms/cms/issues/9842. Now, in Craft 3.7.14, if a mutation is made without specifying a siteId, Craft still throws an error if a disabled site exists.

Note, in the below example, siteId 19 exists, but is disabled. The mutation is not specifying the siteId, so I believe it should assume the default siteId (1), but it still throws an error referencing the disabled site 19. Note this doesn't seem to throw an error when testing via GraphiQL - it seems to require making a GraphQL REST API call to the /api endpoint:

Here's the mutation that is causing this issue:

{operationName: "updateChapterTitle",…}
operationName: "updateChapterTitle"
query: "mutation updateChapterTitle($chapterId: ID, $entryName: String) {\n  save_manuals_chapter_Entry(id: $chapterId, userTitle: $entryName, title: $entryName) {\n    id\n    userTitle\n    title\n    __typename\n  }\n}\n"
variables: {chapterId: 2218, entryName: "Important Safety Info 2"}

And the response from the server:

{"errors":[{"message":"Internal server error","extensions":{"category":"internal"},"locations":[{"line":2,"column":3}],"path":["save_manuals_chapter_Entry"]}],"data":{"save_manuals_chapter_Entry":null}}

And the web.log error:

2021-10-10 19:32:27 [-][-][-][error][yii\base\InvalidConfigException] yii\base\InvalidConfigException: Invalid site ID: 19 in /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/base/Element.php:4328
Stack trace:
#0 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/helpers/ElementHelper.php(605): craft\base\Element->getSite()
#1 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/base/Field.php(373): craft\helpers\ElementHelper::translationKey(Object(craft\elements\Entry), 'language', NULL)
#2 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/services/Elements.php(2885): craft\base\Field->getTranslationKey(Object(craft\elements\Entry))
#3 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/services/Elements.php(2693): craft\services\Elements->_propagateElement(Object(craft\elements\Entry), Array, Object(craft\elements\Entry))
#4 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/services/Elements.php(783): craft\services\Elements->_saveElementInternal(Object(craft\elements\Entry), true, true, true)
#5 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/gql/base/ElementMutationResolver.php(164): craft\services\Elements->saveElement(Object(craft\elements\Entry))
#6 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/gql/resolvers/mutations/Entry.php(56): craft\gql\base\ElementMutationResolver->saveElement(Object(craft\elements\Entry))
#7 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(624): craft\gql\resolvers\mutations\Entry->saveEntry(NULL, Array, Array, Object(GraphQL\Type\Definition\ResolveInfo))
#8 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(550): GraphQL\Executor\ReferenceExecutor->resolveFieldValueOrError(Object(GraphQL\Type\Definition\FieldDefinition), Object(GraphQL\Language\AST\FieldNode), Array, NULL, Object(GraphQL\Type\Definition\ResolveInfo))
#9 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(474): GraphQL\Executor\ReferenceExecutor->resolveField(Object(GraphQL\Type\Definition\ObjectType), NULL, Object(ArrayObject), Array)
#10 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(858): GraphQL\Executor\ReferenceExecutor->GraphQL\Executor\{closure}(Array, 'save_manuals_ch...')
#11 [internal function]: GraphQL\Executor\ReferenceExecutor->GraphQL\Executor\{closure}(Array, 'save_manuals_ch...')
#12 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(860): array_reduce(Array, Object(Closure), Array)
#13 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(490): GraphQL\Executor\ReferenceExecutor->promiseReduce(Array, Object(Closure), Array)
#14 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(263): GraphQL\Executor\ReferenceExecutor->executeFieldsSerially(Object(GraphQL\Type\Definition\ObjectType), NULL, Array, Object(ArrayObject))
#15 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(215): GraphQL\Executor\ReferenceExecutor->executeOperation(Object(GraphQL\Language\AST\OperationDefinitionNode), NULL)
#16 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/Executor/Executor.php(156): GraphQL\Executor\ReferenceExecutor->doExecute()
#17 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/GraphQL.php(162): GraphQL\Executor\Executor::promiseToExecute(Object(GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter), Object(GraphQL\Type\Schema), Object(GraphQL\Language\AST\DocumentNode), NULL, Array, Array, 'updateChapterTi...', NULL)
#18 /srv/app/dci-dg-test/htdocs/vendor/webonyx/graphql-php/src/GraphQL.php(94): GraphQL\GraphQL::promiseToExecute(Object(GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter), Object(GraphQL\Type\Schema), 'mutation update...', NULL, Array, Array, 'updateChapterTi...', NULL, Array)
#19 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/services/Gql.php(544): GraphQL\GraphQL::executeQuery(Object(GraphQL\Type\Schema), 'mutation update...', NULL, Array, Array, 'updateChapterTi...', NULL, Array)
#20 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/controllers/GraphqlController.php(172): craft\services\Gql->executeQuery(Object(craft\models\GqlSchema), 'mutation update...', Array, 'updateChapterTi...', false)
#21 [internal function]: craft\controllers\GraphqlController->actionApi()
#22 /srv/app/dci-dg-test/htdocs/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)
#23 /srv/app/dci-dg-test/htdocs/vendor/yiisoft/yii2/base/Controller.php(181): yii\base\InlineAction->runWithParams(Array)
#24 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/web/Controller.php(189): yii\base\Controller->runAction('api', Array)
#25 /srv/app/dci-dg-test/htdocs/vendor/yiisoft/yii2/base/Module.php(534): craft\web\Controller->runAction('api', Array)
#26 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/web/Application.php(277): yii\base\Module->runAction('graphql/api', Array)
#27 /srv/app/dci-dg-test/htdocs/vendor/yiisoft/yii2/web/Application.php(104): craft\web\Application->runAction('graphql/api', Array)
#28 /srv/app/dci-dg-test/htdocs/vendor/craftcms/cms/src/web/Application.php(262): yii\web\Application->handleRequest(Object(craft\web\Request))
#29 /srv/app/dci-dg-test/htdocs/vendor/yiisoft/yii2/base/Application.php(392): craft\web\Application->handleRequest(Object(craft\web\Request))
#30 /srv/app/dci-dg-test/htdocs/web/index.php(21): yii\base\Application->run()
#31 {main}
2021-10-10 19:32:26 [-][-][-][info][application] Request body: {"operationName":"updateChapterTitle","variables":{"chapterId":2218,"entryName":"Important Safety Info 2"},"query":"mutation updateChapterTitle($chapterId: ID, $entryName: String) {\n  save_manuals_chapter_Entry(id: $chapterId, userTitle: $entryName, title: $entryName) {\n    id\n    userTitle\n    title\n    __typename\n  }\n}\n"}

$_GET = [
    'p' => 'api'
]

$_FILES = []

$_COOKIE = []

Steps to reproduce

  1. Attempt to mutate an entry without specifying siteId in a GraphQL API REST call (not via GraphiQL).
  2. Note it attempts to mutate the last siteId, not the default siteId
  3. Note, if the siteId is disabled, internal server error is thrown.

Additional info

humandevs commented 3 years ago

Actually, I think this is still just the same bug as https://github.com/craftcms/cms/issues/9842, albeit in 3.7.14. Specifying the siteId in the mutation doesn't change the error, and enabling the disabled site in the example allows the mutation to function properly, mutating the siteId specified in the mutation. Thus, it's just that having a disabled site throws an error when attempting a mutation.

Not sure if I should delete this issue and reopen the previous one, or keep this issue open.

andris-sevcenko commented 3 years ago

@humandevs

There was a bug fixed in Craft 3.7.16 that addressed disabled sites being included in site selection sometimes.

So, after that version, you'll probably end up updating the entry on a disabled site.

More to the actual problem you had - if you don't specify the siteId parameter, Craft will default to the current site, not the default site. The current site is mainly determined by the URL.

Either way, I would strongly recommend always to specify a site ID when dealing with multi-site installations.