Symfony, une redirection 302 et une exception sont dans un bateau
Par NiKo le samedi 12 janvier 2008, 20:56 - Dev
- Lien permanent -
10 commentaires -
Tags :
Ah la la. C'est assez rare pour être souligné, mais je viens de me battre avec Symfony pendant près de deux heures avec un problème assez déroutant de prime abord : lorsque dans une action vous faites quelque chose d'aussi anodin que ceci :
php
public function executeBar()
{
try
{
// Some stuff here, if successful redirect user to somewhere else
$this->setFlash('notice', 'Action was successful');
$this->redirect('@whatever_route');
}
catch (Exception $e)
{
$this->getRequest()->setError('errors', 'Something has failed somewhere, sorry dude');
$this->logMessage('Big boo boo occured: '.$e->getMessage(), 'err');
return sfView::SUCCESS;
}
}
Et bien dans ce cas là, la redirection s'opère MAIS l'ensemble du rendu sera tout de même produit et envoyé au navigateur [1] - ce qui peut s'avérer très couteux sur un site internet au final [2]. La raison en est très simple, la méthode redirect() de la classe sfActions met fin à l'execution par ce moyen que l'on peut qualifier de hardi :
php
public function redirect($url, $statusCode = 302)
{
$url = $this->getController()->genUrl($url, true);
if (sfConfig::get('sf_logging_enabled'))
{
$this->getContext()->getLogger()->info('{sfAction} redirect to "'.$url.'"');
}
$this->getController()->redirect($url, 0, $statusCode);
throw new sfStopException();
}
Oui, vous avez bien lu, on interrompt le script en levant une exception, ici de type sfStopException. Le problème, c'est que dans mon exemple précédent, la méthode redirect() est appellée dans un bloc try { } catch { }, et donc l'exception levée est interceptée et l'action n'est au final pas stoppée. Vicieux, hein ?
Pour régler le problème, on peut par exemple toujours effectuer ses appels à la méthode redirect() en dehors d'un block try { } catch { } [3], ou encore tester le type de l'exception levée. Dans notre exemple, cette dernière solution ressemblerait à ça :
php
public function executeFoo()
{
try
{
// Some stuff here, if successful redirect user to somewhere else
/// ...
$this->setFlash('notice', 'Action was successful');
$this->redirect('@whatever_route');
}
catch (sfStopException $e)
{
return sfView::HEADER_ONLY;
}
catch (Exception $e)
{
$this->getRequest()->setError('errors', 'Something has failed somewhere, sorry dude');
$this->logMessage('Big boo boo occured: '.$e->getMessage(), 'err');
return sfView::SUCCESS;
}
}
C'est pas super sexy, mais ça fonctionne, et ça a le mérite de m'inspirer deux morales à cette histoire :
- tester le type des exceptions que l'on catche, c'est bien
- lever une exception pour arrêter un script, c'est super cracra et mériterait éventuellement d'être patché

Edit: Je m'exprime mal, c'est pas super cracra, c'est juste que ça introduit une petite complexité supplémentaire. Mais l'utilité de la chose est bien entendu totalement avérée si on a besoin d'effectuer des opérations particulières avant la fin du script ( ce qui est rarement le cas, enfin chez moi).
Notes
[1] Avec tout ce que ça comporte de requête SQL et de templates calculés pour rien
[2] Et oui bien sûr, faire un redirect() dans un bloc try catch c'est pas forcément le meilleur endroit, mais c'est teeeellement pratique 
[3] Qui sera donc exécuté de toute façon si aucune exception n'est levée.
10 commentaires (Ajouter un commentaire)
Romaric> Humph, tu soulèves une vraie question mais en l'état je ne me sens pas de renvoyer des exceptions pouvant potentiellement nuire à l'expérience utilisateur et d'interrompre sa navigation si je suis capable de les intercepter avant... Là pour le coup, c'est moi qui suis sans doute cracra mais c'est comme ça, j'ai horreur des erreurs 500 affichées pour rien (même si elles sont customizables, effectivement)
N'oublions pas que la valeur de retour d'une méthode d'action est la plupart du temps le dernier endroit où le développeur a encore la main sur les traitements au niveau du contrôleur (non, je ne placerai pas de blocs
try/catchdans les templates ^^.) Et faire un filtre Symfony qui les intercepterait systématiquement me semble un peu lourdingue; là pour le coup. Je préfère logguer l'erreur et afficher un message contextualisé informant l'utilisateur que quelque chose s'est mal déroulé que de l'envoyer sur une bête page d'erreur.Après, les goûts et les couleurs...
Justement, s'il tu catches quelque chose du type natif Exception (dsl, je sais pas le mettre en couleur, lol ^^), il faut peut-être que ça le processus ammenant à l'affichage d'une erreur du client se passe, au cas où il y ait des choses à nettoyer un peu avant d'arrêter tout.
J'imagine que si une exception remonte tout en haut, symfony t'affiche une belle page (ou plutôt pas belle à voir pour l'utilisateur ^^) avec la pile d'appel des fonctions et tout et tout. Y a pas moyen de change juste ce que cette page affiche et de laisser remonter l'exception ?
Ce serait plus propre, non ? D'autant plus que si on imagine qu'avant, tu as fait le tri des exceptions que ton code générait, il ne reste plus que les gros plantages pour lever des Exceptions, qui eux devraient bien envoyer une page d'erreur à l'utilisateur (erreur 500, plantage sur le serveur, non ? il doit bien y avoir moyen de la customiser ^^).
Romaric> En Symdony, retourner
sfRequest::SUCCESSdepuis une méthode d'action ne signifie pas Tout s'est bien passé mais Affiche moi le template par défaut associé à l'action. C'est d'ailleurs pour ça que j'ajoute un message d'erreur à l'objet représentant la requête via sa méthodesetError(), car généralement on factorise ses templates pour n'en avoir qu'un seul à gérer, que les choses se soient bien passées ou non.Par exemple, dans le cas d'un formulaire, il est courant (voire conseillé) en cas d'erreurs détectées dans les valeurs soumises par l'utilisateur de réafficher le formulaire avec les champs en erreur mis en exergue.
Tu trouveras plus d'infos sur la gestion des actions en Symfony par ici, si ça t'intéresse
Sinon pour le cas du catch des exceptions du type natif
Exception, bien entendu que c'est un peu violent, mais tu comprendras aisément qu'à ce niveau du traitement tu ne peux pas renvoyer l'exception, ce qui aboutirait vraisemblablement à l'affichage d'une erreur sur le navigateur du clientJuste un petit truc qui me chagrine, c'est que tu catch d'une part la sfStopException, et d'autre part, n'importe quel type d'exception sans les faire remonter. Est-ce que le framework ne pourrait pas avoir besoin de ces exceptions (par exemple pour fermer des ressources qu'il aurait ouvertes) ?
J'utilise pas spécialement symphony (testé une fois pour voir), mais dans les Exceptions que tu catch, il peut y avoir un gros truc sur lequel le framework a besoin de réagir et tu transformes ça en sfView::SUCCESS (en gros tout va bien si je comprend bien, même si y a une grosse exception).
greg> Oui, c'est ce dont je suis en train de me rendre compte. Après coup, la feature est appréciable (même si je n'ai jamais rencontré de cas concret d'utilisation...)
Plus j'y pense et plus ce qui me dérange(ait) c'est qu'en PHP cette manière d'envisager les choses avec cette /rigueur/ est assez récente. En Java cela ne m'aurait pas choqué le moins du monde. Et oui, les idées reçues ont la vie dures...
Je ne sais pas si l'interruption d'un script ne dois pas être la même chose qu'une exception.
Il se peut qu'on ait à faire un certain nombre de choses avant d'arrêter définitivement le script. Par exemple, certaines ressources n'aiment pas être laissées ouvertes et utiliser une exception me parait être un bon moyen pour tout arrêter proprement.
J'avais déjà été confronté au problème et c'est effectivement un peu déroutant quand on est jamais tombé dessus avant...
Piwaï> Merci pour cette leçon que je saurais retenir avec humilité. Tu reliras cependant mon billet et sa première conclusion
Etait-il réellement nécessaire d'en remettre une couche ? Certainement, et je t'en remercie 
Olivier> Ben une fois que tu connais le truc, tout te parait beaucoup plus simple et propre. En même temps, j'ai jamais vraiment rencontré le cas ou un redirect 302 nécessitait l'execution de code après appel de la méthode correspondante (et il semble que je ne sois pas le seul), mais ça doit sans doute exister, effectivement (filtre, plugins, etc.)
Tu ferais comment pour arrêter l'execution du script ? exit() ? C'est un peu violent.
Perso, je trouve que l'exception sfStopException est bien car on peut l'attraper et détecter ainsi toutes les redirections.
Alors là mon petit coco, je ne suis pas d'accord avec toi !
Je m'en vas te donner mon humble avis.
Tout d'abord, l'information était bien sûr disponible dans le "book" de symfony :
http://www.symfony-project.org/book...
The code located after a forward or a redirect in an action is never executed. You can consider that these calls are equivalent to a return statement. They throw an sfStopException to stop the execution of the action; this exception is later caught by symfony and simply ignored.
Certes, on ne peut retenir l'intégralité du symfony book. Cependant, le fait qu'après un redirect, le code ne soit jamais executé aurait du te mettre la puce à l'oreille.
D'autres part, je me trompe peut-être, mais il me semble que c'est pas très bien de capter 'tout ce qui peut passer'. Il faut connaitre son code, et toutes les exceptions qu'il peut lever, et donc capter celles-ci (avec un traitement différent suivant les exceptions). C'est à ça que sert l'héritage pour les exceptions.
Java a quand même du bon la dessus, vu qu'on est forcé de déclarer les exceptions qui seront 'throw' par la méthode.
Sinon, ne serait-il pas plus propre de faire suivre l'exception du redirect, vu que c'est ce qui se passe si tu ne fais pas de try catch ?
catch (sfStopException $e)
{
throw $e;
}
(je dis ça sans le tester, juste une idée..)
Donc sinon pour ton code, à toi de te créer une classe d'exception spécifique à ton projet, et à faire en sorte que toutes tes exceptions du projet en héritent. Après, plus qu'à catcher cette classe d'exception.
D'autres part, tu peux trouver ici la liste des exceptions de symfony :
http://trac.symfony-project.com/bro...
Hop Hop, c'est fini
. Au passage, ça doit être mon premier commentaire sur ton blog, que je lis pourtant avec plaisir, continue !