Projet

Général

Profil

Development #35029

astregs: en cas de soucis de connexion, self.call provoque un crash 500

Ajouté par Thomas Noël il y a presque 5 ans. Mis à jour il y a plus de 4 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Version cible:
-
Début:
24 juillet 2019
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

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

Révision 676bf063 (diff)
Ajouté par Benjamin Dauvergne il y a plus de 4 ans

utils: raise APIError for wsdl schema loading errors (#35029)

Historique

#2

Mis à jour par Thomas Noël il y a presque 5 ans

#3

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)

#4

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 ?

#5

Mis à jour par Benjamin Dauvergne il y a presque 5 ans

  • Assigné à mis à Benjamin Dauvergne
#6

Mis à jour par Benjamin Dauvergne il y a presque 5 ans

Contre propale, ça corrige le souci dans tous les connecteurs WSDL avec un message plus compréhensible.

#7

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).

#8

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.

#9

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...

#10

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.

#11

Mis à jour par Benjamin Dauvergne il y a presque 5 ans

Ok je me suis fait avoir par le cache de zeep.

#12

Mis à jour par Thomas Noël il y a plus de 4 ans

  • Fichier 0001-signature-forbid-arguments-after-signature-35059.patch supprimé
#13

Mis à jour par Thomas Noël il y a plus de 4 ans

  • Statut changé de Solution proposée à Solution validée
#14

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.

#15

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.

#17

Mis à jour par Thomas Noël il y a plus de 4 ans

  • Statut changé de Solution proposée à Solution validée
#18

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)
#19

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

Formats disponibles : Atom PDF