Skip to content

fix: return null instead of exception for GraphQL Query operation #6118

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

Merged
merged 5 commits into from
Feb 16, 2024

Conversation

Zippovich2
Copy link
Contributor

Q A
Branch? 3.2
Tickets Closes #6072
License MIT
Doc PR -

After upgrading to 3.2 we noticed a new error Item from read provider should be a nullable object. in our projects, when request single resource via GraphQL Query if such resource does not exists with provided id, see #6072.

@@ -68,6 +69,10 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
$item = null;
}

if ($operation instanceof Query && null === $item) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it be at line 86 that we check if the $item is not null? Also can you add a non regression test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it above because it looks like condition !\is_object($item) on line 86 is related to Subscription and Mutation operations logic and $item should be checked on null before it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for test - I did it but unfortunately ApiPlatform\Exception\ItemNotFoundException is outside of GraphQL directory and I cannot mock IriConvertorInterface to throw this exception. Used class_alias to make it work, not sure if it's OK.

@rutek
Copy link

rutek commented Feb 7, 2024

Hello, do you have any updates on this fix? The issue #6072 contains information that an update to the latest 3.2.x should fix it, but the fix seems to not be merged. :D

If I can help you in any way, feel free to contact me. :)

@soyuka soyuka force-pushed the graphql_read_provider_patch branch from 29e27f4 to d0be2ba Compare February 16, 2024 11:45
@soyuka soyuka merged commit 2999d9e into api-platform:3.2 Feb 16, 2024
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

Successfully merging this pull request may close these issues.

3 participants