fixup! wcs: explicite wcs response on WcsApiError (#78967)

passerelle/contrib/toulouse_maelis/models.py

@@ -4486,18 +4486,14 @@ class Subscription(models.Model):
             try:
                 result = wcs_api.post_json(obj.wcs_trigger_payload, [], headers=headers)
             except WcsApiError as e:
-                if e.request_response is not None and e.request_response.status_code == 404:
-                    try:
-                        json_response = json.loads(e.request_response.text)
-                    except ValueError:
-                        self.resource.logger.warning(e)
-                        return
+                if e.json_content is not None and e.request_response.status_code == 404:
                     # stop triggering a removed wcs demand
                     self.resource.logger.info(e)
                     obj.wcs_trigger_date = now()
-                    obj.wcs_trigger_response = json_response
+                    obj.wcs_trigger_response = e.json_content
                     obj.save()
                 else:
+                    # continue triggering on other wcs errors
                     self.resource.logger.warning(e)
                 return
         obj.wcs_trigger_date = now()

passerelle/contrib/toulouse_smart/models.py

@@ -16,7 +16,6 @@
 
 import base64
 import datetime
-import json
 from urllib import parse as urlparse
 from uuid import uuid4
 
@@ -504,9 +503,9 @@ class SmartRequest(models.Model):
         try:
             result = wcs_api.post_json(self.payload, [], headers=headers)
         except WcsApiError as e:
-            try:
-                result = json.loads(e.request_response.text)
-            except ValueError:
+            result = e.json_content
+            if result is None:
+                # continue triggering until we get a json error response
                 return False
         self.result = result
         self.save()

passerelle/utils/wcs.py

@@ -35,14 +35,33 @@ from passerelle.base import signature
 
 
 class WcsApiError(Exception):
-    def __init__(self, *args, **kwargs):
-        self.request_exception = kwargs.pop('request_exception')
-        self.__dict__.update(kwargs)
-        super().__init__(*args, **kwargs)
+    def __init__(self, *args, url='', request_exception=None):
+        self.url = url
+        self.request_exception = request_exception
+        self.request_response = getattr(self.request_exception, 'response', None)
+        self.content = getattr(self.request_response, 'content', None)
+        try:
+            # parse the wcs response when available
+            self.json_content = json.loads(self.content)
+        except (ValueError, TypeError):
+            self.json_content = None
+        super().__init__(*args)
 
-    @property
-    def request_response(self):
-        return getattr(self.request_exception, 'response', None)
+    def __str__(self):
+        message = ' '.join(self.args)
+        message += ' (url=%s' % self.url
+        if self.request_response is not None:
+            message += ', status_code=%s' % self.request_response.status_code
+        if self.json_content:
+            message += ', json_content=%s' % json.dumps(self.json_content, ensure_ascii=False)
+        elif self.content:
+            message += ', content=%s' % self.content
+        elif self.request_response is not None:
+            message += ', request_response=%s' % repr(self.request_response)
+        elif self.request_exception is not None:
+            message += ', request_exception=%s' % repr(self.request_exception)
+        message += ')'
+        return message
 
 
 class JSONFile:
@@ -579,13 +598,12 @@ class WcsApi:
             response = self.requests.get(final_url)
             response.raise_for_status()
         except requests.RequestException as e:
-            content = getattr(getattr(e, 'response', None), 'content', None)
-            raise WcsApiError('GET request failed', final_url, e, content, request_exception=e)
+            raise WcsApiError('GET request failed', url=complete_url, request_exception=e)
         else:
             try:
                 return response.json()
             except ValueError as e:
-                raise WcsApiError('Invalid JSON content', final_url, e)
+                raise WcsApiError('Invalid JSON content: %s' % e, url=complete_url)
 
     def post_json(self, data, *url_parts, headers=None):
         headers = headers or {'content-type': 'application/json'}
@@ -606,13 +624,12 @@ class WcsApi:
             response = self.requests.post(final_url, data=json.dumps(data), headers=headers)
             response.raise_for_status()
         except requests.RequestException as e:
-            content = getattr(getattr(e, 'response', None), 'content', None)
-            raise WcsApiError('POST request failed', final_url, e, content, request_exception=e)
+            raise WcsApiError('POST request failed', url=complete_url, request_exception=e)
         else:
             try:
                 return response.json()
             except ValueError as e:
-                raise WcsApiError('Invalid JSON content', final_url, e)
+                raise WcsApiError('Invalid JSON content: %s' % e, url=complete_url)
 
     @property
     def roles(self):

tests/test_toulouse_maelis.py

@@ -18,6 +18,7 @@ import datetime
 import json
 import logging
 import os
+import textwrap
 from unittest import mock
 
 import pytest
@@ -11306,7 +11307,7 @@ def test_trigger_wcs_service_error(family_service, activity_service, con, app, f
     )
 
 
-def test_trigger_wcs_api_error(family_service, activity_service, wcs_service, con, app, freezer):
+def test_trigger_wcs_api_error(family_service, activity_service, wcs_service, con, app, freezer, caplog):
     family_service.add_soap_response('readFamily', get_xml_file('R_read_family_for_subscription.xml'))
     activity_service.add_soap_response('getPersonUnitInfo', get_xml_file('R_get_person_unit_info.xml'))
     activity_service.add_soap_response('addPersonUnitBasket', get_xml_file('R_add_person_unit_basket.xml'))
@@ -11344,6 +11345,12 @@ def test_trigger_wcs_api_error(family_service, activity_service, wcs_service, co
     )
     con.hourly()
     assert len([x for x in wcs_service.calls if '/hooks/' in x.request.url]) == 1
+    assert caplog.records[-1].levelno == logging.WARNING
+    assert textwrap.wrap(caplog.records[-1].message, 80) == [
+        'POST request failed (url=https://wcs.example.com/api/forms/exemple-inscription-',
+        'loisirs-1/12/hooks/update_subscription/?orig=passerelle, status_code=403,',
+        'json_content={"err": 1, "err_class": "Access denied", "err_desc": null})',
+    ]
     subscription = con.subscription_set.get(wcs_form_number='13-12')
     assert subscription.trigger_status() == 'triggering'
 
@@ -11355,6 +11362,12 @@ def test_trigger_wcs_api_error(family_service, activity_service, wcs_service, co
     )
     con.hourly()
     assert len([x for x in wcs_service.calls if '/hooks/' in x.request.url]) == 2
+    assert caplog.records[-1].levelno == logging.WARNING
+    assert textwrap.wrap(caplog.records[-1].message, 80) == [
+        'POST request failed (url=https://wcs.example.com/api/forms/exemple-inscription-',
+        'loisirs-1/12/hooks/update_subscription/?orig=passerelle,',
+        "request_exception=ConnectionError('No address associated with hostname'))",
+    ]
     subscription = con.subscription_set.get(wcs_form_number='13-12')
     assert subscription.trigger_status() == 'triggering'
 
@@ -11367,6 +11380,12 @@ def test_trigger_wcs_api_error(family_service, activity_service, wcs_service, co
     )
     con.hourly()
     assert len([x for x in wcs_service.calls if '/hooks/' in x.request.url]) == 3
+    assert caplog.records[-1].levelno == logging.WARNING
+    assert textwrap.wrap(caplog.records[-1].message, 80) == [
+        'POST request failed (url=https://wcs.example.com/api/forms/exemple-inscription-',
+        'loisirs-1/12/hooks/update_subscription/?orig=passerelle, status_code=500,',
+        "content=b'plop')",
+    ]
     subscription = con.subscription_set.get(wcs_form_number='13-12')
     assert subscription.trigger_status() == 'triggering'
     assert subscription.wcs_trigger_response is None
@@ -11380,6 +11399,12 @@ def test_trigger_wcs_api_error(family_service, activity_service, wcs_service, co
     )
     con.hourly()
     assert len([x for x in wcs_service.calls if '/hooks/' in x.request.url]) == 4
+    assert caplog.records[-1].levelno == logging.WARNING
+    assert textwrap.wrap(caplog.records[-1].message, 80) == [
+        'POST request failed (url=https://wcs.example.com/api/forms/exemple-inscription-',
+        'loisirs-1/12/hooks/update_subscription/?orig=passerelle, status_code=404,',
+        "content=b'not a json content')",
+    ]
     subscription = con.subscription_set.get(wcs_form_number='13-12')
     assert subscription.trigger_status() == 'triggering'
     assert subscription.wcs_trigger_response is None
@@ -11393,6 +11418,12 @@ def test_trigger_wcs_api_error(family_service, activity_service, wcs_service, co
     )
     con.hourly()
     assert len([x for x in wcs_service.calls if '/hooks/' in x.request.url]) == 5
+    assert caplog.records[-1].levelno == logging.INFO
+    assert textwrap.wrap(caplog.records[-1].message, 80) == [
+        'POST request failed (url=https://wcs.example.com/api/forms/exemple-inscription-',
+        'loisirs-1/12/hooks/update_subscription/?orig=passerelle, status_code=404,',
+        'json_content={"err": 1, "err_class": "Page non trouvée", "err_desc": null})',
+    ]
     subscription = con.subscription_set.get(wcs_form_number='13-12')
     assert subscription.trigger_status() == 'triggered'
     assert subscription.wcs_trigger_response == {'err': 1, 'err_class': 'Page non trouvée', 'err_desc': None}

tests/test_toulouse_smart.py

@@ -29,7 +29,7 @@ import httmock
 import lxml.etree as ET
 import pytest
 from django.utils.encoding import force_str
-from requests.exceptions import ReadTimeout
+from requests.exceptions import ConnectionError, ReadTimeout
 
 import tests.utils
 from passerelle.base.models import Job
@@ -831,7 +831,7 @@ def test_update_intervention_job_wrong_service(mocked_uuid, app, smart, wcs_serv
     ['/api/forms/foo/2/hooks/update_intervention/', UPDATE_INTERVENTION_QUERY, WCS_RESPONSE_ERROR, 403],
 )
 @mock.patch("django.db.models.fields.UUIDField.get_default", return_value=UUID)
-def test_update_intervention_job_wcs_error(mocked_uuid, app, smart, wcs_service):
+def test_update_intervention_job_wcs_error(mocked_uuid, app, smart, wcs_service, caplog):
     resp = app.post_json(URL + 'create-intervention/', params=CREATE_INTERVENTION_PAYLOAD)
     assert not resp.json['err']
 
@@ -851,6 +851,40 @@ def test_update_intervention_job_wcs_error(mocked_uuid, app, smart, wcs_service)
     ['/api/forms/foo/2/hooks/update_intervention/', UPDATE_INTERVENTION_QUERY, 'bla', 500],
 )
 @mock.patch("django.db.models.fields.UUIDField.get_default", return_value=UUID)
+def test_update_intervention_job_wcs_error_not_json(mocked_uuid, app, freezer, smart, wcs_service):
+    freezer.move_to('2021-07-08 00:00:00')
+    resp = app.post_json(URL + 'create-intervention/', params=CREATE_INTERVENTION_PAYLOAD)
+    assert not resp.json['err']
+
+    url = URL + 'update-intervention?uuid=%s' % str(UUID)
+    resp = app.post_json(url, params=UPDATE_INTERVENTION_PAYLOAD)
+    assert not resp.json['err']
+    job = Job.objects.get(method_name='update_intervention_job')
+    assert job.status == 'registered'
+    smart_request = smart.wcs_requests.get(uuid=UUID).smart_requests.get()
+    assert smart_request.result is None
+
+    freezer.move_to('2021-07-08 00:00:03')
+    smart.jobs()
+    job = Job.objects.get(method_name='update_intervention_job')
+    assert job.status == 'registered'
+    assert job.update_timestamp > job.creation_timestamp
+    smart_request = smart.wcs_requests.get(uuid=UUID).smart_requests.get()
+    assert smart_request.result is None
+
+
+@mock_response(
+    ['/v1/type-intervention', None, INTERVENTION_TYPES],
+    ['/v1/intervention', CREATE_INTERVENTION_QUERY, get_json_file('create_intervention')],
+    [
+        '/api/forms/foo/2/hooks/update_intervention/',
+        UPDATE_INTERVENTION_QUERY,
+        None,
+        500,
+        ConnectionError('No address associated with hostname'),
+    ],
+)
+@mock.patch("django.db.models.fields.UUIDField.get_default", return_value=UUID)
 def test_update_intervention_job_transport_error(mocked_uuid, app, freezer, smart, wcs_service):
     freezer.move_to('2021-07-08 00:00:00')
     resp = app.post_json(URL + 'create-intervention/', params=CREATE_INTERVENTION_PAYLOAD)