Lighthouse: Allow to write logs on production

Created on 28 Oct 2019  ·  13Comments  ·  Source: nuwave/lighthouse

Describe the solution you'd like

When APP_DEBUG=true an we have this configuration ...

```
'debug' => \GraphQL\Error\Debug::INCLUDE_DEBUG_MESSAGE | \GraphQL\Error\Debug::RETHROW_UNSAFE_EXCEPTIONS,
````

... errors are written to storage/logs/.

When app is in production (so, APP_DEBUG=false) errors will not place on storage/logs.

Describe alternatives you've considered

Any config setting to always write errors on storage/logs even if APP_DEBUG=false.

enhancement

Most helpful comment

What if we just make the "easy" fix and when an error is thrown we call report method?
This way the exception is added passed to the Laravel Exception handler so it can be reported to bugsnag or whatever.

@robsontenorio Would that solve your problem?

All 13 comments

We would have to build some custom error handling around that.

As of now, we don't deal with Exceptions ourselves.

  • In production graphql-php catches them and makes sure it still returns a proper GraphQL response
  • When debugging/developing, the Exceptions are rethrown and Laravel deals with them

What you are asking is a combination of both. We have to make sure we are always returning proper GraphQL responses in production but would also like Exceptions to be handled in whatever way Laravel is set up.

Might be related to https://github.com/nuwave/lighthouse/issues/1006

Yes, maybe related.

I agree with catch error and return proper response. But, without properly
logging mechanism in production we can lost useful info for a proactive
product maintenance.

In other words, in production, only clients will aware about errors, not
developers.

More context about not predictable errors:

  • Database connectivity
  • Trying to get property from null object
  • ...

What if we just make the "easy" fix and when an error is thrown we call report method?
This way the exception is added passed to the Laravel Exception handler so it can be reported to bugsnag or whatever.

@robsontenorio Would that solve your problem?

That would be a good option! I hope you guys get my point about missing logging mechanisms.

@robsontenorio Yeah it makes totally sense. Just haven't actually thought about that happening yet 👍

We use an external service 'Sentry' for error logging on production and currently no errors are shown in sentry when lighthouse throws an internal server error. This is very annoying. Any tips on how to report these errors ?

@Christophvh How did you implement sentry integration? E.g. I added following code to main exceptions handler in App\Exceptions\Handler::report:

if (app()->bound('sentry')
    && $this->shouldReport($exception)
    && config('app.env') !== 'local'
) {
    $user = Auth::user();
    if (!empty($user)) {
        configureScope(function (Scope $scope) use ($user): void {
            $scope->setUser([
                'id'    => $user->id,
                'email' => $user->email
            ]);
        });
    }
    app('sentry')->captureException($exception);
}

And it works in production for me.
If you want to catch only GraphQL Errors, you may bind custom error handler via lighthouse config, where you then can send exception to sentry

@lorado

This is my Handler:

    /**
     * Report or log an exception.
     *
     * @return void
     */
    public function report(Exception $exception)
    {
        if (app()->bound('sentry') && $this->shouldReport($exception)) {
            app('sentry')->captureException($exception);
        }

        parent::report($exception);
    }

Works perfect for normal Laravel apps, but with Lighthouse, no errors are ever shown in Sentry unless we make a specific error handler just for Lighthouse. We don't want to set APP_DEBUG=true in production obviously.

Yes, that is the “issue” pointed by @Christophvh

We currently make our own exception handler and added a extra if condition for internal errors:

<?php

namespace App\GraphQL;

use Closure;
use GraphQL\Error\Error;
use Illuminate\Support\Facades\Log;
use Nuwave\Lighthouse\Execution\ExtensionErrorHandler;
use Nuwave\Lighthouse\Exceptions\RendersErrorsExtensions;

class ErrorHandler extends ExtensionErrorHandler
{
    /**
     * Handle Exceptions that implement Nuwave\Lighthouse\Exceptions\RendersErrorsExtensions
     * and add extra content from them to the 'extensions' key of the Error that is rendered
     * to the User.
     *
     * @param \GraphQL\Error\Error $error
     * @param \Closure             $next
     *
     * @return array
     */
    public static function handle(Error $error, Closure $next): array
    {
        $underlyingException = $error->getPrevious();

        if ($underlyingException instanceof RendersErrorsExtensions) {
            // Reconstruct the error, passing in the extensions of the underlying exception
            $error = new Error(
                $error->message,
                $error->nodes,
                $error->getSource(),
                $error->getPositions(),
                $error->getPath(),
                $underlyingException,
                $underlyingException->extensionsContent()
            );
        }
        if (Error::CATEGORY_INTERNAL === $error->getCategory()) {
            Log::error($error->message);
        }

        return $next($error);
    }
}

In case you want to report errors to Sentry and also would want to report the Sentry event ID to your frontend (for user feedback) you could consider something like this:

<?php

namespace App\Exceptions;

use Closure;
use Sentry\State\Scope;
use GraphQL\Error\Debug;
use GraphQL\Error\Error;
use GraphQL\Error\FormattedError;
use Nuwave\Lighthouse\Execution\ErrorHandler;

class GraphQLHandler implements ErrorHandler
{
    /**
     * This function receives all GraphQL errors and may alter them or do something else with them.
     *
     * Always call $next($error) to keep the Pipeline going. Multiple such Handlers may be registered
     * as an array in the config.
     *
     * @param \GraphQL\Error\Error $error
     * @param \Closure             $next
     *
     * @return array
     */
    public static function handle(Error $error, Closure $next): array
    {
        if (app()->bound('sentry')) {
            $eventId = null;

            app('sentry')->withScope(function (Scope $scope) use ($error, &$eventId) {
                $scope->setExtra('details', FormattedError::createFromException($error, Debug::INCLUDE_DEBUG_MESSAGE));
                $scope->setExtra('clientSafe', $error->isClientSafe());

                $eventId = app('sentry')->captureException($error);
            });

            if (!empty($eventId)) {
                $error = new Error(
                    $error->getMessage(),
                    $error->getNodes(),
                    $error->getSource(),
                    $error->getPositions(),
                    $error->getPath(),
                    $error->getPrevious(),
                    array_merge($error->getExtensions(), compact('eventId'))
                );
            }
        }

        return $next($error);
    }
}

This does report any error to Sentry, even when you would make a typo or throw some exception that can be caught by the front-end so you might want to filter the exceptions that are reported to filter out some noise :) You can do this for example by checking the error category and only report if it's not internal (like in the example from @Christophvh).

    public static function handle(Error $error, Closure $next): array
    {
+        if ($error->getCategory() !== Error::CATEGORY_INTERNAL) {
+            return $next($error);
+        }

        // The rest of the method as above...

I think we should make use of the native error handling in Laravel. Calling report() seems like the right way to propagate the errors, we just need to figure out which ones we should report by default.

See https://github.com/nuwave/lighthouse/pull/1303 for discussing the concrete implementation i plan to add.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

vine1993 picture vine1993  ·  3Comments

spawnia picture spawnia  ·  4Comments

alexwhb picture alexwhb  ·  4Comments

a-ssassi-n picture a-ssassi-n  ·  3Comments

Leuloch picture Leuloch  ·  3Comments