Allow to configure exception catching for schema execution

[N-Maas]

Currently, any exception that is raised in a resolver is catched and converted to a GraphQLError, which is returned in the errors part of the result (see strawberry/schema/execute.py).

However, this is not always desirable. For example, the error message might contain sensitive information about the server that should not be accessible to a client. Instead, the server should return a 500 HTTP error (which would work more or less automatically if the exception is not catched). So we need a possibility to tell strawberry that it should not catch all exceptions, but only specific ones. This could mean catching only GraphQLError, or allowing the user to define a set of exceptions that should be catched.

[jkimbo]

@N-Maas I think there are 2 issues that you have outlined here and they are slightly different:

1. Error messages might contain sensitive information about the server

This is true and we'd like to make it easier to hide error messages using extensions (see https://github.com/strawberry-graphql/strawberry/issues/1155). Until then here is a custom extension that only allows certain errors to be included in the response:

from graphql import GraphQLError
from strawberry.extensions import Extension

class VisibleError(Exception):
    pass

class MaskErrors(Extension):
    def on_request_end(self):
        result = self.execution_context.result
        if result.errors:
            processed_errors = []
            for error in result.errors:

                if error.original_error and not isinstance(error.original_error, VisibleError):
                    processed_errors.append(
                        GraphQLError(
                            message="Unexpected error.",
                            nodes=error.nodes,
                            source=error.source,
                            positions=error.positions,
                            path=error.path,
                            original_error=None
                        )
                    )
                else:
                    processed_errors.append(error)

            result.errors = processed_errors

@strawberry.type
class Query:
    @strawberry.field
    def my_field(self) -> str:
        raise VisibleError("This error message is visible to the client")

    @strawberry.field
    def my_second_field(self) -> str:
        raise Exception("This error message is renamed to 'Unexpected error.'")

schema = strawberry.Schema(Query, extensions=[MaskErrors])

(I'll make a PR for this soon so that you can use it directly from strawberry)

2. The server should return a 500 if there are any/some errors

The GraphQL spec actually defines how a server should deal with errors and it explicitly says that errors should be caught and added to the "errors" response: https://spec.graphql.org/June2018/#sec-Errors-and-Non-Nullability There is also a draft spec for "GraphQL over HTTP" which defines the status codes that the server should return: https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#status-codes

So it's unlikely that Strawberry will support returning 500 errors if there is an error or selectively catching specific exceptions since it contravenes the spec.

---

I hope thats helpful and that I characterised your question correctly. Let me know if I've missed anything.

[N-Maas]

@jkimbo Thank you, that should probably work for us!

I was not aware of the error handling definition in the GraphQL spec, in this case it is of course better to adhere to the spec.

[blueyed]

I'm not sure if everything should be converted to an error really (according to the spec): the case at hand is due to library upgrades, where it looks like this:

Traceback (most recent call last):
  File "…/project/.venv/lib/python3.10/site-packages/graphql/execution/execute.py", line 521, in execute_field
    result = resolve_fn(source, info, **args)
  File "…/project/.venv/lib/python3.10/site-packages/strawberry_django_plus/optimizer.py", line 573, in resolve
    ret = _next(root, info, *args, **kwargs)
  File "…/project/.venv/lib/python3.10/site-packages/strawberry/schema/schema_converter.py", line 495, in _resolver
    return _get_result(_source, strawberry_info, **kwargs)
  File "…/project/.venv/lib/python3.10/site-packages/strawberry/schema/schema_converter.py", line 489, in _get_result
    return self.config.default_resolver(_source, field.python_name)
AttributeError: 'NoneType' object has no attribute 'foo'

I do not think it makes sense to convert this into GraphQLError("'NoneType' object has no attribute 'foo'", locations=[SourceLocation(line=3, column=13)], path=['foo']).

Additionally due to the nature of the inner try / except it will lose information about the stack until the try-except block, which could be mitigated (https://stackoverflow.com/questions/6086976/how-to-get-a-complete-exception-stack-trace-in-python), but is not trivial/native.

When used in test suites etc it is much easier to work with exceptions in this case also, of course.

Therefore I think not converting all exceptions in general would be better, and at least should be made configurable.

code ref: https://github.com/strawberry-graphql/strawberry/blob/702c29fdedb9e563cd521c8fefc11700b26cd753/strawberry/schema/execute.py#L160-L180

[jkimbo]

@blueyed the code you have linked to is not where execution errors are captured, it's where the query is parsed. Execution errors are captured in the graphql-core library (https://github.com/graphql-python/graphql-core/blob/c214c1d93a78d7d1a1a0643b1f92a8bf29f4ad14/src/graphql/execution/execute.py#L512-L559) and it's not simple to override that behaviour (because it's part of the spec).

[blueyed]

@jkimbo indeed. In the case at hand it comes via:

  [33] …/project/app/api/test_bar.py(90)test_baz()
       result = schema.execute_sync(query, variable_values={"urn": foo.urn})
  [34] …/project/.venv/lib/python3.10/site-packages/strawberry/schema/schema.py(231)execute_sync()
       result = execute_sync(
  [35] …/project/.venv/lib/python3.10/site-packages/strawberry/schema/execute.py(193)execute_sync()
       result = original_execute(
  [36] …/project/.venv/lib/python3.10/site-packages/graphql/execution/execute.py(1030)execute()
       result = exe_context.execute_operation(operation, root_value)
  [37] …/project/.venv/lib/python3.10/site-packages/graphql/execution/execute.py(353)execute_operation()
       return (
  [38] …/project/.venv/lib/python3.10/site-packages/graphql/execution/execute.py(431)execute_fields()
       result = self.execute_field(
  [39] …/project/.venv/lib/python3.10/site-packages/graphql/execution/execute.py(557)execute_field()
       error = located_error(raw_error, field_nodes, path.as_list())
  [40] …/project/.venv/lib/python3.10/site-packages/graphql/error/located_error.py(50)located_error()
       return GraphQLError(message, nodes, source, positions, path, original_error)
> [41] …/project/.venv/lib/python3.10/site-packages/graphql/error/graphql_error.py(132)__init__()
       self.original_error = original_error

I've come up with the following to raise any original errors, which works good for tests, but could also make sense in general then:

class Schema(StrawberrySchema):
    def process_errors(
        self,
        errors: "list[GraphQLError]",
        execution_context: "ExecutionContext" = None,
    ) -> None:
        super().process_errors(errors, execution_context)

        for error in errors:
            err = getattr(error, "original_error")
            if err:
                raise err