magento2: Graphql Exception "Category image not found" if image doesn't exist shouldn't error the entire request
Summary (*)
When trying to get a list of categories, or even only one category, if “image” field is filled but the image can’t be found on filesystem the request will fail with “Error: Category image not found.” This shouldn’t be the correct behavior as not being able to render an image for a category isn’t an unrecoverable error, and we’ve been able to handle in frontend this kind of errors for years, either be using a fallback or by leveraging other solutions. Plus, when displaying a list of categories an image missing shouldn’t block the whole categories being displayed by erroring the request.
Examples (*)
- Upload an image to a category
- Delete it from FS
- Run any categories Graphql query including “image” attribute
- Watch it fail by “Error: Category image not found.”
In our case, this was the query:
const QUERY = gql`
query GetLines {
categories {
items {
children {
name
children {
name
children {
name
fast_shipping
url_path
image
}
}
}
}
}
}
`;
Proposed solution
Remove entirely this exception: https://github.com/magento/magento2/blob/1807f70756b521240c06eb556802422307cabbb5/app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Image.php#L70-L72
Please provide Severity assessment for the Issue as Reporter. This information will help during Confirmation and Issue triage processes.
- Severity: S0 - Affects critical data or functionality and leaves users with no workaround.
No workaround that we can think of can be applied to recover from this error apart from re-uploading the missing image.
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 1
- Comments: 17 (5 by maintainers)
@cpartica I was thinking that we should pass a message with the success response stating that “Category X” image wasn’t found along with the placeholder image but I don’t think that’s a good idea anymore, as returning a placeholder makes it clear enough (and passing a message in a graphql’s success response should not be the correct way to deal with this kind of errors).
I will open a PR replacing the exception with fallback placeholder image, that should be enough.
@iba-1 can you elaborate on the “standard graphql responses”? What would you expect such a doc to have? We don’t have such a doc, but we should. We can collaborate and create one. Please find me on community slack. Anybody else is welcome to join.
I think for this case, we have to replace this with the default image placeholder. A query can’t fail unless there’s an unrecoverable error.
Thank you, @shikhamis11, for bringing this to my attention