Development #35029
astregs: en cas de soucis de connexion, self.call provoque un crash 500
0%
Description
ça m'étonne un peu, mais sur un appel SOAP qui plante pour un pépin réseau, on reçoit une 500 avec ce payload :
{"err_class": "requests.exceptions.ConnectionError", "err_desc": "('Connection aborted.', error(\"(104, 'ECONNRESET')\",))", "data": null, "err": 1}
Pour moi il faut que ça réponde en 200, on a le "err:1" qui dit que Passerelle a fait son travail mais que c'est l'appli en face qui a un soucis.
Fichiers
Révisions associées
Historique
Mis à jour par Thomas Noël il y a presque 5 ans
- Fichier 0001-astregs-handle-low-level-connection-errors-35029.patch 0001-astregs-handle-low-level-connection-errors-35029.patch ajouté
- Tracker changé de Support à Bug
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
Un peu perdu parce qu'il n'y a pas de self.call uniquement du
client = get_client(...) response = client.service.op(...)
Mais là leur truc est bien down, ça ferme la connexion dès que ça reçoit du HTTP qu'il ne veut pas (c'est à dire tout pour l'instant) :
$ openssl s_client -host test-ws-astre-gs.departement06.fr -port 443 -servername test-ws-astre-gs.departement06.fr CONNECTED(00000003) depth=2 C = FR, O = Dhimyotis, CN = Certigna verify return:1 depth=1 C = FR, O = DHIMYOTIS, OU = 0002 48146308100036, 2.5.4.97 = NTRFR-48146308100036, CN = Certigna Wild CA verify return:1 depth=0 C = FR, L = NICE, O = DEPARTEMENT des alpes maritimes, 2.5.4.97 = NTRFR-22060001900016, OU = 0002 22060001900016, CN = *.departement06.fr verify return:1 --- Certificate chain 0 s:/C=FR/L=NICE/O=DEPARTEMENT des alpes maritimes/2.5.4.97=NTRFR-22060001900016/OU=0002 22060001900016/CN=*.departement06.fr i:/C=FR/O=DHIMYOTIS/OU=0002 48146308100036/2.5.4.97=NTRFR-48146308100036/CN=Certigna Wild CA 1 s:/C=FR/O=DHIMYOTIS/OU=0002 48146308100036/2.5.4.97=NTRFR-48146308100036/CN=Certigna Wild CA i:/C=FR/O=Dhimyotis/CN=Certigna 2 s:/C=FR/O=Dhimyotis/CN=Certigna i:/C=FR/O=Dhimyotis/CN=Certigna --- Server certificate -----BEGIN CERTIFICATE----- MIII2zCCBsOgAwIBAgIQHaLP5fWvwnJOsR1MPz8NeDANBgkqhkiG9w0BAQsFADB5 MQswCQYDVQQGEwJGUjESMBAGA1UECgwJREhJTVlPVElTMRwwGgYDVQQLDBMwMDAy IDQ4MTQ2MzA4MTAwMDM2MR0wGwYDVQRhDBROVFJGUi00ODE0NjMwODEwMDAzNjEZ MBcGA1UEAwwQQ2VydGlnbmEgV2lsZCBDQTAeFw0xOTA0MTYwODM3NTNaFw0yMTA0 MTUwODM3NTNaMIGgMQswCQYDVQQGEwJGUjENMAsGA1UEBwwETklDRTEoMCYGA1UE CgwfREVQQVJURU1FTlQgZGVzIGFscGVzIG1hcml0aW1lczEdMBsGA1UEYQwUTlRS RlItMjIwNjAwMDE5MDAwMTYxHDAaBgNVBAsMEzAwMDIgMjIwNjAwMDE5MDAwMTYx GzAZBgNVBAMMEiouZGVwYXJ0ZW1lbnQwNi5mcjCCASIwDQYJKoZIhvcNAQEBBQAD ggEPADCCAQoCggEBAN4yzNK+F1YoxMJVFdfqpc5CFNNyD13xQpUtSVq25Ar3YCJ3 NMWrP25mKoufxKUCw13qYkbR4G+b9pr46Me5+bfEgBetJYicEGoV4qF0qSIRKX3U fQE79v/1+ZFg4GAgnZF7Cm7Kzk2lb9dwMjFoBrAru1tvSfjFa5Utb1DUfQ36wnBD 1VH2SjhAewYP5zwLeR3eSa235zhHslfVrOEfJCi+h8Jgh7lP+FfWy2ZEsCki5OlD iDmCSt6FEC1hlRAx8QOqX9h5dIoRKPIu2ryRnqtuG7TCE8FSbkxvsBPQZ0aiA61C KUDQdpIcp6nhXzq/lpoGL4vnzJ3JUy4j2x5mz0kCAwEAAaOCBDUwggQxMAkGA1Ud EwQCMAAwDgYDVR0PAQH/BAQDAgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEF BQcDAjBdBgNVHR8EVjBUMCegJaAjhiFodHRwOi8vY3JsLmNlcnRpZ25hLmZyL3dp bGRjYS5jcmwwKaAnoCWGI2h0dHA6Ly9jcmwuZGhpbXlvdGlzLmNvbS93aWxkY2Eu Y3JsMIHUBggrBgEFBQcBAQSBxzCBxDAyBggrBgEFBQcwAoYmaHR0cDovL2F1dG9y aXRlLmNlcnRpZ25hLmZyL3dpbGRjYS5kZXIwNAYIKwYBBQUHMAKGKGh0dHA6Ly9h dXRvcml0ZS5kaGlteW90aXMuY29tL3dpbGRjYS5kZXIwKgYIKwYBBQUHMAGGHmh0 dHA6Ly93aWxkY2Eub2NzcC5jZXJ0aWduYS5mcjAsBggrBgEFBQcwAYYgaHR0cDov L3dpbGRjYS5vY3NwLmRoaW15b3Rpcy5jb20wHQYDVR0OBBYEFOATkq5koc9rQze0 bPaGFWcavlPLMB8GA1UdIwQYMBaAFI58F7e898O0mNa2NZkiKsTDniRnMC8GA1Ud EQQoMCaCEiouZGVwYXJ0ZW1lbnQwNi5mcoIQZGVwYXJ0ZW1lbnQwNi5mcjBUBgNV HSAETTBLMAgGBmeBDAECAjA/BgsqgXoBgTECBwECATAwMC4GCCsGAQUFBwIBFiJo dHRwczovL3d3dy5jZXJ0aWduYS5mci9hdXRvcml0ZXMvMIIB9gYKKwYBBAHWeQIE AgSCAeYEggHiAeAAdgDuS723dc5guuFCaR+r4Z5mow9+X7By2IMAxHuJeqj9ywAA AWolS61nAAAEAwBHMEUCIQDg6iglrSv0e5+t3lxvUzGIFEAQrof6j5cST6KDbcq3 twIgQ4/j8t66YjpHZCElTcmNZLaOsNkDOG5VCZbu18nerYoAdgBvU3asMfAxGdiZ AKRRFf93FRwR2QLBACkGjbIImjfZEwAAAWolS7H6AAAEAwBHMEUCIQCNWwHjO9Nc FreU66V7kehRu9kSTIBm7FqRhtDwnCBoJgIgNoN1AcWWjaxfKaeyPncefaMAvGyR ftO0ghIzCZd8BdkAdwBc3EOS/uarRUSxXprUVuYQN/vV+kfcoXOUsl7m9scOygAA AWolS7RrAAAEAwBIMEYCIQCKj8MLK3Knerjp4nHEhgphAuKRrNT3ISOacldprQsj HwIhAJ5p7xLNaNYCUCPSnaT9VjmtsMHjCWfBYqVOyv8njhWcAHUA9lyUL9F3MCIU VBgIMJRWjuNNExkzv98MLyALzE7xZOMAAAFqJUu00wAABAMARjBEAiB/OzOegRzb VBKaWtckWhXHL88dVhAt4NvNSD5Ed82PmQIgYV6E4j/+uk787ZLdlJn6D6vyHnl0 LOTswOTVgYzXpw8wDQYJKoZIhvcNAQELBQADggIBAIXbVcti95jNR4aMoDGeKSux HPg1Bfu5ZROGv6rUKB363zzBXfZEGALVgIYriLTLCeymXl3j2fXhsygITigNqef9 YPsXYmPoSXbjrbnMy61LDEPYpa8m6CdZO7bVHi/IXNnlB41+/Ga1GgKhZKRbV2Lc bmsjFxn8topOQ210W0Z/ybXsbCywqUSNGEUF2N/krdrCCtNPWrgQcDy33IEh4rS6 xY4QJFlK41KoHIfbuxtpvJ4AzsZU8jDqIpT79g8MMBlOv5yAmHEYf0Ht+H+WpU0V zCpaCectnbobE/VUPDnfXGgSDv08I50PN7OsxTgGQ2+W5JNx66AqBTEx5EMjNRNU jYg5n9JyfabFftR0KnSC5Q28Oj6Cjj7B0SNEBb2JxLsJB0jbAJaEhGS8fnc7XMFr HBAjBGrtwcFQ3g93bF+1lWZozoZ/l+cEhLh4QRXCT7BiXT+B9dREFTENxs3ngWtt 9URdvrvOSQE7A1wkhOqClBwgdMgFQWkG/JMtWGynFEEgV8k+dzi5V39IyoFWhK8N gPM3udouuXhHF/XmVynsHSSxUnLduGLtrMYRtnScIwLqSvMUlaQ8PlUm4Ats/LJn wxh5gi/TDIFaE/Pse4HAT/hv2yPS1TwkEPrf8gl4y6TsUCn9IH09R9K59gcTTwKQ f4eI8thJerQKvyObf6XM -----END CERTIFICATE----- subject=/C=FR/L=NICE/O=DEPARTEMENT des alpes maritimes/2.5.4.97=NTRFR-22060001900016/OU=0002 22060001900016/CN=*.departement06.fr issuer=/C=FR/O=DHIMYOTIS/OU=0002 48146308100036/2.5.4.97=NTRFR-48146308100036/CN=Certigna Wild CA --- No client certificate CA names sent Peer signing digest: SHA256 Server Temp Key: DH, 1024 bits --- SSL handshake has read 5469 bytes and written 408 bytes Verification: OK --- New, TLSv1.2, Cipher is DHE-RSA-AES256-GCM-SHA384 Server public key is 2048 bit Secure Renegotiation IS supported Compression: NONE Expansion: NONE No ALPN negotiated SSL-Session: Protocol : TLSv1.2 Cipher : DHE-RSA-AES256-GCM-SHA384 Session-ID: A2982432F7DD99178B615F965E25409F220F1D632CA3EC07E69EA4162EB590AF Session-ID-ctx: Master-Key: B17401C7244A938D0B88F26985439E0B3D547D4AC5DD8AFAD2E0F64FF67FA8638A4592EFA09E551A8FF0780667C0EAC0 PSK identity: None PSK identity hint: None SRP username: None Start Time: 1563976108 Timeout : 7200 (sec) Verify return code: 0 (ok) Extended master secret: no --- GET / HTTP/1.1 read:errno=104
Ici y a tout qui foire, le check_status sur l'URL de base (dont je ne sais pas vraiment ce qu'elle retourne normalement) mais aussi la récupération des WSDL (qui se faisant or boucle SOAP request/response fait qu'on se prend des erreurs requests alors que zeep a aussi une exception TransportError)..
Ce que je propose c'est de transformer les erreurs requests dans notre méthode SOAPTransport._load_remote_data() en APIError claires1.
1 On peut pas vraiment faire mieux, la chaîne actuelle d'initialisation d'un client c'est
- Client.__init__(location) => self.wsdl = Document(location, self.transport)
- Document.__init__(location, transport) => self.transport = transport
- document = self._get_xml_document(location)
- get_xml_document(location) => load_external(location, self.transport)
- load_external(location, transport) => transport.load(location)
- load(location) => self._load_remote_data(url)
Mis à jour par Thomas Noël il y a presque 5 ans
Benjamin Dauvergne a écrit :
Un peu perdu parce qu'il n'y a pas de self.call uniquement du ...
Pas bien compris ta remarque.
Dans astregs il y a un self.call qui lance toute la mécanique zeep, et qui fait le try/except sur les erreurs zeep. J'y ajoute juste ici les potentiels crash low level... c'est un peu sale mais bon, comme zeep ne semble pas attraper ces choses là.
Dans notre méthode SOAPTransport._load_remote_data() en APIError claires
Ça ce n'est que la partie récupération wsdl, il faudrait aussi attraper le post fait ensuite sur le service.
Et plutôt que d'y faire une APIError, pourquoi ne pas émettre une zeep.exception.TransportError ?
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
- Fichier 0001-utils-raise-APIError-for-wsdl-schema-loading-errors-.patch 0001-utils-raise-APIError-for-wsdl-schema-loading-errors-.patch ajouté
- Tracker changé de Bug à Development
Contre propale, ça corrige le souci dans tous les connecteurs WSDL avec un message plus compréhensible.
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
Thomas Noël a écrit :
Benjamin Dauvergne a écrit :
Un peu perdu parce qu'il n'y a pas de self.call uniquement du ...
Pas bien compris ta remarque.
Mon dépôt était trop vieux je n'avais pas de call, mais ça ne change rien à mon analyse.
Dans astregs il y a un self.call qui lance toute la mécanique zeep, et qui fait le try/except sur les erreurs zeep. J'y ajoute juste ici les potentiels crash low level... c'est un peu sale mais bon, comme zeep ne semble pas attraper ces choses là.
Uniquement sur la résolution du WSDL.
Dans notre méthode SOAPTransport._load_remote_data() en APIError claires
Ça ce n'est que la partie récupération wsdl, il faudrait aussi attraper le post fait ensuite sur le service.
Si on a pas récupérer le WSDL on risque pas d'avoir de POST (d'ailleurs tu verras que dans mon test modifié du tien je surcharge le POST différemment pour être sûr qu'il n'y en a pas). Je n'ai pas trouvé de trace où ça plante sur le POST, ça plante toujours avant ou ça ne plante pas (ça ne veut pas dire que c'est impossible, mais là c'est pas le cas). Aussi toutes RequestException sur un POST/SOAP sera convertie en TransportError, ce ne sont que les GET de résolution du .wsdl et de ses dépendances qui ne sont pas converties.
Et plutôt que d'y faire une APIError, pourquoi ne pas émettre une zeep.exception.TransportError ?
Parce que la résolution WSDL c'est du meta-SOAP ça fait bizarrement pas partie de la norme (dans les specs SOAP à aucun moment ça n'indique comment récupérer le WSDL, t'es sensé l'avoir point), je suis plus à l'aise de remonter directement à haut niveau les erreurs de résolution du WSDL, on comprend très bien ce qui se passe en fait, pas besoin de laisser aux connecteurs le soin de le gérer. En vrai j'envisage de faire disparaître TransportError des connecteurs quand j'aurai implémenté un SOAPField.
Après ça pourrait être bien de le mettre en cache juste pour les perfs (parce que quand c'est down de toute façon on aura l'erreur au POST ensuite, mais sous forme TransportError).
Mis à jour par Thomas Noël il y a presque 5 ans
A priori ça fait planter les tests bien comme il faut, dommage parce que l'idée semble effectivement intéressante.
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
Thomas Noël a écrit :
A priori ça fait planter les tests bien comme il faut, dommage parce que l'idée semble effectivement intéressante.
Bizarre chez moi ça marchait avant que je pousse...
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
Benjamin Dauvergne a écrit :
Thomas Noël a écrit :
A priori ça fait planter les tests bien comme il faut, dommage parce que l'idée semble effectivement intéressante.
Bizarre chez moi ça marchait avant que je pousse...
Et ça marche toujours, je cherche.
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
- Fichier 0002-utils-raise-APIError-for-wsdl-schema-loading-errors-.patch 0002-utils-raise-APIError-for-wsdl-schema-loading-errors-.patch ajouté
- Fichier 0001-signature-forbid-arguments-after-signature-35059.patch ajouté
Ok je me suis fait avoir par le cache de zeep.
Mis à jour par Thomas Noël il y a plus de 4 ans
- Fichier
0001-signature-forbid-arguments-after-signature-35059.patchsupprimé
Mis à jour par Thomas Noël il y a plus de 4 ans
- Statut changé de Solution proposée à Solution validée
Mis à jour par Thomas Noël il y a plus de 4 ans
- Statut changé de Solution validée à Solution proposée
Bon en fait non, comme je suis un peu tatasse j'aurai bien vu une exception genre SOAPRequestException(APIError) (ou autre nom), histoire de pouvoir la catcher spécifiquement dans certains appels SOAP qui auraient le droit de foirer, dans certains endpoint. Ok ça existe nulle part pour l'instant oui je sais mais bon.
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
Thomas Noël a écrit :
Bon en fait non, comme je suis un peu tatasse j'aurai bien vu une exception genre SOAPRequestException(APIError) (ou autre nom), histoire de pouvoir la catcher spécifiquement dans certains appels SOAP qui auraient le droit de foirer, dans certains endpoint. Ok ça existe nulle part pour l'instant oui je sais mais bon.
Je suis parfaitement d'accord avec ça par contre, je vais le faire.
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Fichier 0001-utils-raise-APIError-for-wsdl-schema-loading-errors-.patch 0001-utils-raise-APIError-for-wsdl-schema-loading-errors-.patch ajouté
Rebasé sur master, classe SOAPError(APIError) ajoutée.
Mis à jour par Thomas Noël il y a plus de 4 ans
- Statut changé de Solution proposée à Solution validée
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit 676bf063e19923215a145823d44812a25a1daa10 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Wed Jul 24 16:30:51 2019 +0200 utils: raise APIError for wsdl schema loading errors (#35029)
Mis à jour par Frédéric Péters il y a plus de 4 ans
- Statut changé de Résolu (à déployer) à Solution déployée
utils: raise APIError for wsdl schema loading errors (#35029)