Unfriendly/unhelpful error handling in RegistrationController::confirmAction

[laczoka]

In RegistrationController::confirmAction:

if (null === $user) { throw new NotFoundHttpException(sprintf('The user with confirmation token "%s" does not exist', $token)); }

throws a NotFoundHttpException if someone clicks on the confirmation links for the second time which I don't think is very helpful and could be confusing to the user. (making him believe that sg went wrong, whereas in reality it didn't).

Instead there should be a standard template saying that you're either confirmed your registration or your registration code is invalid.

[laczoka]

Can provide a patch if the issue is accepted as valid.

[strayer]

If I could vote on this I'd vote on this... I get several e-mails with this Exception per day by our monitoring and I can't really see if there is a problem or the user just clicked on the link again because he didn't bookmark the page. (Seriously, users do this. And if they see an 404 its not a good user experience)

[strayer]

So, I was updating to Symfony 2.2 today and this was bugging me for months.

Since nothing is happening here, this is a quick and horrible workaround for the issue:

Override the controller (see the FOSUserBundle docs about that) and add this action:

    public function confirmAction($token)
    {
        try {
            return parent::confirmAction($token);
        }
        catch (NotFoundHttpException $e) {
            $url = $this->container->get('router')->generate('fos_user_security_login');
            return new RedirectResponse($url);
        }
    }

It will redirect the user directly to the login page instead of throwing an Exception.

[fogs]

We are having the same issue here..

[lecajer]

Work around

class NotFoundExceptionsListener
{
    private $router;
    private $session;

    public function __construct(Router $router, Session $session)
    {
        $this->router = $router;
        $this->session = $session;
    }

    public function onKernelException(GetResponseForExceptionEvent $event)
    {
        if (HttpKernel::MASTER_REQUEST != $event->getRequestType()) {
            // don't do anything if it's not the master request
            return;
        }

        $e = $event->getException();

        if($e instanceof NotFoundHttpException) {
            if(strpos($e->getMessage(), 'The user with confirmation token') !== false && strpos($e->getMessage(), 'does not exist') !== false) {
                $this->session->setFlash('warning', 'trans');
                $event->setResponse(new RedirectResponse($this->router->generate('fos_user_security_login')));
                $event->stopPropagation();
            }
        }
    }
}

[joaotnlima]

 /**
 * Receive the confirmation token from user email provider, login the user
 */
public function confirmAction($token)
{
    try {
        return parent::confirmAction($token);
    }
    catch (NotFoundHttpException $e) {
        $url = $this->container->get('router')->generate('fos_user_security_login');
        return new RedirectResponse($url);
    }
}

This is a better way of leading with the problem... otherwise the stack of exceptions in the app will grow... and that it will degrade the performance.

the problem was that you've changed the interface so here is the solution which I consider the better aproach in terms of scalability:

 /**
 * Receive the confirmation token from user email provider, login the user
 */
public function confirmAction(Request $request, $token)
{
    try {

        return parent::confirmAction($request, $token);

    }
    catch (NotFoundHttpException $e) {
        $url = $this->container->get('router')->generate('fos_user_security_login');
        return new RedirectResponse($url);
    }
}