cakephp-error-email icon indicating copy to clipboard operation
cakephp-error-email copied to clipboard

EmailThrowableTrait access to context / overriding default algorithms

Open smarek opened this issue 6 years ago • 1 comments

This is enhancement proposal, solving simple app-specific issue. If the error is thrown in loop code (ie. generating "Undeffined offset" in loop code), this will generate as many emails, as there are loop iterations.

Currently $cacheKey is determined from throwable class, message and code concatenated and cleaned.

If app is to implement workaround for such situation, it would need to copy relevant code (generating $cacheKey), and for example replace the exception message with filename and line-number, that produced said throwable.

This has 2 possible solutions:

  • Add option ErrorEmail.throttleAlgorithm with numerical values, that identify the algorithm that is used to throttle (this can be expanded later, and each algorithm should be implemented as standalone method in EmailThrowableTrait so it can be overriden for app-specific cases)
  • Update trait interface to protected function _appSpecificSkipThrottle($throwable, $context = []), which would allow passing extra context data to app-specific handler (again, this proposed update can be applied to all _appSpecific* methods), context could in this case include cacheKey, but better would be to extract cacheKey generation to separate function, so it can be overriden ultimately

Second proposed approach, would also help greatly if ErrorHandlerMiddleware.handleException called the trait method _emailThrowable with $request and $response so these could be passed as context in trait methods, or be ultimately accessible from app-specific trait implementation through other methods

This whole proposal is to prevent users of this lib, to blindly copy-paste / override whole functions (such as _throttle or emailThrowable) from current EmailThrowableTrait, to achieve goals similar to described ones, which could result in fatal breaks in future, as soon as the lib code gets updated.

I'm actually thinking about providing both means (extract cacheKey algorithm generation, extract throttle algorithm no.1, and providing context in trait via methods _getRequest and _getResponse), so if you're cool with that, let me know, I'll push relevant changes for you to review.

smarek avatar Feb 25 '20 16:02 smarek

Since I wanted to comply with current state of things, I've made a modified ErrorHandlerMiddleware to check on the session context, before deciding, what to do with the issue. See the example below:

This way, basic exceptions, like NotFoundException, MissingRouteException or AuthSecurityException will not be sent, when there is no user in the request context (in this case, session storage for cakephp 3.x AuthComponent)

<?php

namespace App\Middleware;

use App\Model\Entity\User;
use App\Traits\EmailThrowableTrait;
use Cake\Controller\Exception\AuthSecurityException;
use Cake\Http\Exception\InvalidCsrfTokenException;
use Cake\Http\Exception\NotFoundException;
use Cake\Http\ServerRequest;
use Cake\Routing\Exception\MissingRouteException;
use Throwable;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;

class ErrorHandlerMiddleware extends \ErrorEmail\Middleware\ErrorHandlerMiddleware
{
    use EmailThrowableTrait;

    private function middlewareSkipEmail(?Throwable $exception, ?ServerRequest $request, ?ResponseInterface $response)
    {
        $userdata = $request->getSession()->read('Auth.User');
        if (!empty($exception) && (empty($userdata) || !($userdata instanceof User))) {
            // skip emailing for these errors if it's unauthenticated (no user) request
            $exception_class = get_class($exception);
            switch ($exception_class) {
                case MissingRouteException::class:
                case InvalidCsrfTokenException::class:
                case NotFoundException::class:
                case AuthSecurityException::class:
                    return true;
            }
        }
        return false;
    }

    /**
     * Add email funcitonality to handle exception
     *
     * @param Exception $exception The exception to handle.
     * @param ServerRequestInterface $request The request.
     * @param ResponseInterface $response The response.
     * @return ResponseInterface A response
     */
    public function handleException($exception, $request, $response)
    {
        if (!($request instanceof ServerRequest) || !$this->middlewareSkipEmail($exception, $request, $response)) {
            // Add emailing throwable functionality
            $this->emailThrowable($exception);
            // Use parent funcitonality
        }
        return $this->_callParent($exception, $request, $response);
    }
}

smarek avatar Jun 08 '20 07:06 smarek