Skip to content

Return custom error message #513

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

Closed
jonatasbaldin opened this issue Jul 27, 2017 · 14 comments
Closed

Return custom error message #513

jonatasbaldin opened this issue Jul 27, 2017 · 14 comments

Comments

@jonatasbaldin
Copy link

Hi there!

I'd like to know the better way to raise an exception on a Mutation. For example, while trying to create a User with an email address that already exists.

if email
    user = User.objects.filter(email=email).first()

    if user:
       # ?

return CreateUser(username=username, password=password)

I know that I can raise Exception('message'), but is this the best way? Can we do it differently?

Thanks!

@ariannedee
Copy link

I usually return a GraphQLError.

from graphql import GraphQLError
...
raise GraphQLError('That email already exists')

It takes a few more arguments: nodes, stack, source, and positions, but I have never used these.

@BossGrand
Copy link
Member

BossGrand commented Aug 16, 2017

This kind of depends on what you're trying to achieve. Are you trying to send back a message to the front end user? Or are you trying to send a error message to a fellow developer?

Based on the context of your question I would guess you are trying to send a message to the front end user. If that is the case then you need to catch the exception, or capture the error message and return it as part of the payload of your mutation.

Now this gets tedious if you want to achieve this for all your mutations. What I've done is written my own Mutation Class that extends ClientIDMutation

I'll provide the solution I came up with. I only needed simple single string error messages. If you need to return more complex error details then this will need to be expanded.

class CustomClientIDMutationMeta(ClientIDMutationMeta):
    ''' We have to subclass the metaclass of ClientIDMutatio and inject the errors field.
        we do this because ClientIDMutation subclasses do not inherit the fields on it.
    '''
    def __new__(mcs, name, bases, attrs):
        attrs['errors'] = String()
        return super().__new__(mcs, name, bases, attrs)



class CustomClientIDMutation(ClientIDMutation, metaclass=CustomClientIDMutationMeta):
    ''' Custom ClientIDMutation that has a errors  @fields.'''
    @classmethod
    def mutate(cls, root, args, context, info):
        try:
            return super().mutate(root, args, context, info)

        except MutationException as e:
            return cls(errors=str(e))

Also note that I've created a custom Exception Object called MutationException

class MutationException(Exception):
    '''A Mutation Exception is an exception that is raised
       when an error message needs to be passed back to the frontend client
       our mutation base class will catch it and return it appropriately
    '''
    pass

This solution has worked really nicely for my needs because anytime I need to send a validation message I just have to raise a MutationException and it will propagate back to the frontend user nicely

@jonatasbaldin
Copy link
Author

jonatasbaldin commented Aug 16, 2017

Thanks! Really awesome usage! I think we should include the GraphQLError on the Graphene docs.

@japrogramer
Copy link

@BossGrand wouldn't the user need to query for errors on the mutation?
If the user does a mutation without requesting the errors field, they won't see anything.

@jkimbo
Copy link
Member

jkimbo commented Mar 17, 2018

I'm going to close this issue since it's stale. However just to add: I've found that modeling expected errors as part of your mutation response is essential. Your client needs to know what to do if your mutation fails. To do that I've found that modeling your response types as unions works really well since it allows you explicitly model the errors you're expecting. So in a createUser mutation similar to yours @jonatasbaldin you can do this:

class CreateUserFailUsernameExists(graphene.ObjectType):
	suggested_alternatives = graphene.List(graphene.String)
	error_message = graphene.String(required=True)


class CreateUserFailOther(graphene.ObjectType):
	error_message = graphene.String(required=True)


class CreateUserSuccess(graphene.ObjectType):
	user = graphene.Field(User, required=True)


class CreateUserPayload(graphene.Union):
    class Meta:
        types = (CreateUserFailUsernameExists, CreateUserFailOther, CreateUserSuccess)


class CreateUser(graphene.Mutation):
	class Arguments:
		username = graphene.String(required=True)
		password = graphene.String(required=True)
	
	Output = CreateUserPayload

	def mutate(root, info, username, password):
    	if User.objects.filter(username=username).exists():
			return CreateUserFailUsernameExists(
				error_message="Username already exists",
				suggested_alternatives=get_alternatives(username)
			)
		try:
			user = create_user(username, password)
			return CreateUserSuccess(user=user)
		except:
			return CreateUserFailOther(error_message="Something went wrong")

Then in your client your mutation query becomes:

mutation createUser($username, String!, $password: String!) {
	createUser(username: $username, password: $password) {
		__typename
		... on CreateUserFailUsernameExists {
			suggestedAlternatives
		}
		... on CreateUserFailOther {
			errorMessage
		}
		... on CreateUserSuccess {
			user {
				id
			}
		}
	}
}

Then you just need to check the __typename value to figure out if there was an error (and what kind) or if the mutation succeed.

@jkimbo jkimbo closed this as completed Mar 17, 2018
@reverland
Copy link

just raise GraphQL may not a good idea? for I find the error directly raised in mutation wont be even catched? so tests with assertRaises will never work.

In [9]: try:
   ...:     client.execute(query_string)
   ...: except:
   ...:     pass
   ...:
---------------------------------------------------------------------------
GraphQLLocatedError                       Traceback (most recent call last)
~/.pyenv/versions/3.6.2/lib/python3.6/site-packages/graphql/execution/executor.py in complete_value_catching_error(exe_context, return_type, field_asts, info, result)
    328     try:
    329         completed = complete_value(
--> 330             exe_context, return_type, field_asts, info, result)
    331         if is_thenable(completed):
    332             def handle_error(error):

~/.pyenv/versions/3.6.2/lib/python3.6/site-packages/graphql/execution/executor.py in complete_value(exe_context, return_type, field_asts, info, result)
    381     # print return_type, type(result)
    382     if isinstance(result, Exception):
--> 383         raise GraphQLLocatedError(field_asts, original_error=result)
    384
    385     if isinstance(return_type, GraphQLNonNull):

GraphQLLocatedError: custom_error

@dreamawakening
Copy link

The problem is if you raise an error (such as GraphQLError) it shows up in your logs, but I only want to display an error to the user.

Does anyone have a solution for this?

@Siecje
Copy link

Siecje commented Apr 24, 2019

@dspacejs With

graphene==1.4.1
graphene-django==1.3
graphql-core==1.1

It was using logger.exception() https://github.com/graphql-python/graphql-core/blob/master/graphql/execution/executor.py#L452
I was able to add a filter on the graphql.execution.executor logger by looking at the exception type.

class GraphQLLogFilter(logging.Filter):
    def filter(self, record):
        exc_type, exc, _ = record.exc_info
        custom_error = isinstance(exc, GraphQLError)
        if custom_error:
            return False
        return True

LOGGING = {
    'version': 1,
    'disable_existing_loggers': False,
    'handlers': {
        'console': {
            'level': 'DEBUG',
            'class': 'logging.StreamHandler',
        },
    },
    # Prevent graphql exception from displaying in console
    'filters': {
        'graphql_log_filter': {
            '()': GraphQLLogFilter,
        }
    },
    'loggers': {
        'graphql.execution.executor': {
            'level': 'WARNING',
            'handlers': ['console'],
            'filters': ['graphql_log_filter'],
        },
    },
}

With

graphene==2.1.3
graphene-django==2.2.0
graphql-core==2.1

It is no longer an exception but an error log and the logger has changed to graphql.execution.utils. https://github.com/graphql-python/graphql-core/blob/master/graphql/execution/utils.py#L155

class GraphQLLogFilter(logging.Filter):
    def filter(self, record):
        if 'graphql.error.located_error.GraphQLLocatedError:' in record.msg:
            return False
        return True

LOGGING = {
    'version': 1,
    'disable_existing_loggers': False,
    'handlers': {
        'console': {
            'level': 'DEBUG',
            'class': 'logging.StreamHandler',
        },
    },
    # Prevent graphql exception from displaying in console
    'filters': {
        'graphql_log_filter': {
            '()': GraphQLLogFilter,
        }
    },
    'loggers': {
        'graphql.execution.utils': {
            'level': 'WARNING',
            'handlers': ['console'],
            'filters': ['graphql_log_filter'],
        },
    },
}

I believe that this is no longer used and could be removed.
https://github.com/graphql-python/graphql-core/blob/master/graphql/execution/executor.py#L452

@dreamawakening
Copy link

dreamawakening commented Apr 25, 2019

@Siecje brilliant solution, it works great. Thanks!

Edit: a few issues though, with this solution there's no way to conditionally log errors.

Whenever any exception is thrown, it's received as a GraphQLLocatedError in the record.msg arg in GraphQLLogFilter.filter(). The record arg doesn't contain any data on what type of exception was originally thrown. This means there's no way to conditionally log errors.

Therefore all you can do is filter all exceptions because they're all received as GraphQLLocatedErrors. This isn't desired because then legitimate errors thrown during any GraphQL query/mutation won't be logged, instead of only filtering user-facing exceptions you only want to present to the user (i.e GraphQLError).

Another problem: it doesn't seem to prevent GraphQLErrors from being logged during testing:

An error occurred while resolving field Query.user
Traceback (most recent call last):
  File "/home/daniel/Dev/projects/social-matchmaking/server/users/schema_resolvers.py", line 22, in resolve_user
    return User.objects.get(pk=kwargs.get('pk'))
  File "/home/daniel/Dev/projects/social-matchmaking/server/.venv/lib/python3.7/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/daniel/Dev/projects/social-matchmaking/server/.venv/lib/python3.7/site-packages/django/db/models/query.py", line 408, in get
    self.model._meta.object_name
users.models.User.DoesNotExist: User matching query does not exist.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/daniel/Dev/projects/social-matchmaking/server/.venv/lib/python3.7/site-packages/graphql/execution/executor.py", line 447, in resolve_or_error
    return executor.execute(resolve_fn, source, info, **args)
  File "/home/daniel/Dev/projects/social-matchmaking/server/.venv/lib/python3.7/site-packages/graphql/execution/executors/sync.py", line 16, in execute
    return fn(*args, **kwargs)
  File "/home/daniel/Dev/projects/social-matchmaking/server/users/schema_resolvers.py", line 24, in resolve_user
    raise GraphQLError(str(error))
graphql.error.base.GraphQLError: User matching query does not exist.

I inserted a breakpoint into GraphQLLogFilter.filter(), and it looks like it hits filter() after the error is logged into the console (so it never has the chance to be filtered).

I also tried using logging.disable() on the different logging levels and that didn't do anything either. Any ideas?

@Siecje
Copy link

Siecje commented Apr 25, 2019

What is different about testing? Which logger did you disable? The new logger is 'graphql.execution.utils'.

The current code isn't logging an exception, it is logging a message with level error.

So yes it means we can't filter based on exception type. Which is what I was doing in the early version.

@GitRon
Copy link

GitRon commented May 10, 2019

@dspacejs Is it safe to not log all the GraphQLError? I image if such an error occurs within graphene or graphene-django then I wouldn't log it, right?

@Siecje
Copy link

Siecje commented Aug 1, 2019

@GitRon You are correct that this will not log any exceptions. I thought it was only GraphQLErrors that were being logged as GraphQLLocatedError.

if 'graphql.error.located_error.GraphQLLocatedError:' in record.msg: will always be True for any exception from graphene.

@matt-dalton
Copy link

Sorry to bring up an old issue, but I have a question...

The Union approach (suggested by @jkimbo ) seems nice, but the problem I see with implementing it (at least for us) is that when you add in an error case you lose backwards compatibility, no? We have a lot of mutations already, so tricky to add in retrospect.

To deal with this, have you used this mutation-style from the start for every single mutation? Without doing that, it becomes difficult to add a user error later down the line when you decide you need one.

@m3nu
Copy link

m3nu commented Jun 5, 2020

Ended up with this filter function. Before I always missed a few exceptions from different places, which was confusing.

from graphql import GraphQLError

class GraphQLLogFilter(logging.Filter):
    """
    Filter GraphQL errors that are intentional. See
    https://github.com/graphql-python/graphene/issues/513
    """
    def filter(self, record):
        if record.exc_info:
            etype, _, _ = record.exc_info
            if etype == GraphQLError:
                return None
        if record.stack_info and 'GraphQLError' in record.stack_info:
            return None
        if record.msg and 'GraphQLError' in record.msg:
            return None
        
        return True

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

No branches or pull requests