pix
pix copied to clipboard
[TECH] Utiliser le logger du service monitoring pour les erreurs d'authentification OIDC
:unicorn: Problème
Aujourd'hui il est difficile de comprendre la source des erreurs liés à l'authentification des utilisateurs pole emploi car il y a peu d'information dans nos logger.
:robot: Solution
Utiliser le logErrorWithCorrelationIds
du service monitoring-tools.js
plutôt que le logger
classique, afin d'avoir un peu plus de contexte lorsque ces erreurs apparaîssent (ex: l'userId, la requestId).
:rainbow: Remarques
Proposition : ne pas tester les loggers. Ce n'est pas une fonctionnalité de l'application et on est surement amené à les modifier pour de nouveaux besoins (comme ici). Les tests n'apportent ici pas vraiment de plus value je trouve 🤔
:100: Pour tester
Vérifier la CI passe
I'm deploying this PR to these urls:
- App (.fr): https://app-pr4947.review.pix.fr
- App (.org): https://app-pr4947.review.pix.org
- Orga: https://orga-pr4947.review.pix.fr
- Certif: https://certif-pr4947.review.pix.fr
- Admin: https://admin-pr4947.review.pix.fr
- API: https://api-pr4947.review.pix.fr/api/
Please check it out!
Je ne trouve pas ça vraiment plus simple 🤔
Dans le code actuel on a toute la manière de récupérer les info de l'utilisateur dans le même fichier. A mon sens ça évite de jongler avec 2 fichiers différents pour comprendre ce qu'il se passe.
Le service oidc-authentication-service
c'est un peu la brique technique qui détaille comment notre flow oidc marche et le usecase authenticate-oidc-user.js
c'est plus le listing "métier" pour authentifier un utilisateur externe. Le usecase dit "va me chercher les infos de cet utilisateur ci", il n'a pas besoin de savoir le détail de comment techniquement on va chercher ces infos là, c'est plus la responsabilité de la couche technique (donc du service).
Bon après ça sort un peu de la PR , mais perso je trouve que le code tel qu'il est répartie bien les responsabilités :)