Optimize views by danthewildcat · Pull Request #1439 · graphql-python/graphene-django (original) (raw)

Choose a reason for hiding this comment

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

Took me a minute to realize why this new op_error logic above is included, and I see it's trying to provide a helpful error message to the user for the specific cases that get_operation_ast ends up returning None, which seems nice.

Question though: is it always true that execute will necessarily error in the cases that get_operation_ast returns None? If not, I worry that this new logic will introduce errors in some cases where we didn't used to error. Before, POST requests seemed to still call execute even if the AST result was None. Based on a search in the graphql-core repo, it seems that get_operation_ast is just a utility and not used internally, so I can't easily detect if there are similar scenarios that raise errors.