Custom error handling

[ChaplyginWezom]

Hi! I've tried to write my own custom error handler, so I can override message given from Laravel's ModelNotFoundException.
But, in my case exception is being thrown inside of @canFind directive, then catched and completely replaced with generic Webonyx Error

            $modelOrModels = $this->directiveArgValue('findOrFail', true)
                    ? $enhancedBuilder->findOrFail($findValue)
                    : $enhancedBuilder->find($findValue);
            } catch (ModelNotFoundException $modelNotFoundException) {
                throw new Error($modelNotFoundException->getMessage());
            }

Is it intended behaviour due to some security concerns or was it just overlooked feature? Because I saw that in some earlier version of Lighthouse this exception was never catched for @can directive

[spawnia]

See #1225.

[spawnia]

Adding the previous exception makes sense. I am open to a pull request that changes this.

[ChaplyginWezom]

Unfortunately, I'm struggling to pass respective tests, as linking previous exception makes it not client-safe

$this->isClientSafe = $previous instanceof ClientAware
    ? $previous->isClientSafe()
    : $previous === null;

From my understanding the only way to circumvent this is to provide respective error handler for ModelNotFoundException from the start. But implementing such feature will build stronger dependency on Laravel framework. So this is a dead end on my side...

[spawnia]

We don't have to throw GraphQL\Error, we can introduce a custom ClientSafeModelNotFoundException.

[ChaplyginWezom]

You're right. And then, with passed previous exception, developer could fine-tune desired message format in own handler.
The last question I have: should we change default message of a new exception so it can be a truly client-safe exception?)

I mean default model exception maybe do not expose sensitive information, but renders class name with namespace, which is probably not very nice for production environment. Or it would be an unnecessary side-effect in taking such precautions?

[spawnia]

I am a bit wary if massaging the Laravel error message or customizing it is useful or necessary. Let's do the quick fix first, add a test that covers it, then we can maybe iterate on better messages.