Downgrade some expected "already exists" errors to info-level logging (#958)

assets/app/vue/defines.ts

@@ -1,5 +1,5 @@
-export const APP_PASSWORD_SUPPORT_URL = 'https://support.tb.pro/hc/articles/46805020320275-App-password-support';
 export const OTHER_APPS_SUPPORT_URL = 'https://support.tb.pro/hc/articles/45584951087635-Using-Thundermail-with-other-clients';
+export const APP_PASSWORD_SUPPORT_URL = 'https://support.tb.pro/hc/articles/46805020320275-Create-an-app-password';
 export const WHAT_IS_IMAP_SUPPORT_URL = 'https://support.tb.pro/hc/articles/46108465880467-What-is-IMAP';
 export const WHAT_IS_JMAP_SUPPORT_URL = 'https://support.tb.pro/hc/articles/46109214513939-What-is-JMAP';
 export const WHAT_IS_SMTP_SUPPORT_URL = 'https://support.tb.pro/hc/articles/46106891841043-What-is-SMTP';

src/thunderbird_accounts/authentication/api.py

@@ -5,6 +5,7 @@
 from thunderbird_accounts.authentication.permissions import CanCreateTestAllowListEntries
 from enum import StrEnum
 from urllib.parse import quote
+import logging
 
 from django.conf import settings
 from mozilla_django_oidc.contrib.drf import OIDCAuthentication
@@ -311,7 +312,12 @@ def sign_up(request: Request):
             status=400,
         )
     except ImportUserError as ex:
-        sentry_sdk.capture_exception(ex)
+        if ex.is_already_exists and not can_register_with_username(partial_username):
+            logging.info(
+                f'Dupe signup suppressed (status={ex.status_code or "none"}, error={ex.error_code or "unknown"})',
+            )
+        else:
+            sentry_sdk.capture_exception(ex)
         return Response(
             {
                 'error': ex.error_desc if ex.error_desc else _('There was an unknown error, please try again later.'),

src/thunderbird_accounts/authentication/clients.py

@@ -339,22 +339,14 @@ def import_user(
                 },
             )
         except RequestException as exc:
-            sentry_sdk.capture_exception(exc)
-
-            try:
-                error_data: dict = json.loads(exc.response.content.decode())
-            except (TypeError, JSONDecodeError):
-                # Could not determine explicit error information
-                error_data = {}
-
             # Note: status_code == 409 means the user already exists on keycloak which we should have caught before this
             # function call.
-            raise ImportUserError(
-                username=username,
-                error=f'Error<{exc.response.status_code}>: {exc.response.content.decode()}',
-                error_code=error_data.get('error', f'status-{exc.response.status_code}'),
-                error_desc=error_data.get('error_description', error_data.get('errorMessage')),
-            )
+            error = ImportUserError.from_request_exception(exc, username=username)
+
+            if not error.is_already_exists:
+                sentry_sdk.capture_exception(exc)
+
+            raise error
 
         # Request returns an empty body on a 201 success, so retrieve pkid from location.
         # ex/ {'Location': 'http://keycloak:8999/admin/realms/tbpro/users/39a7b5e8-7a64-45e3-acf1-ca7d314bfcec', ... }

src/thunderbird_accounts/authentication/exceptions.py

@@ -1,5 +1,10 @@
+import json
 from typing import Optional
 
+from requests.exceptions import RequestException
+
+POSTGRES_DUPLICATE_KEY_MARKER = 'duplicate key value violates unique constraint'
+
 
 class KeycloakError(RuntimeError):
     """Generic error"""
@@ -31,13 +36,15 @@ def __str__(self):
 class ImportUserError(KeycloakError):
     username: str
     error: str
+    """Format an error usefully if the user already exists in Keycloak"""
 
     def __init__(
         self,
         error,
         username: Optional[str] = None,
         error_code: Optional[str] = None,
         error_desc: Optional[str] = None,
+        status_code: Optional[int] = None,
         *args,
         **kwargs,
     ):
@@ -48,6 +55,44 @@ def __init__(
         # Structured error from keycloak
         self.error_code = error_code
         self.error_desc = error_desc
+        self.status_code = status_code
+
+    @classmethod
+    def from_request_exception(cls, exc: RequestException, *, username: Optional[str] = None):
+        response = exc.response
+        status_code = None
+        response_content = ''
+        error_data = {}
+
+        if response is not None:
+            status_code = response.status_code
+            response_content = response.content.decode(errors='replace')
+            try:
+                error_data = json.loads(response_content)
+            except (TypeError, json.JSONDecodeError):
+                error_data = {}
+
+        if response_content:
+            error = f'Error<{status_code}>: {response_content}'
+        else:
+            error = f'Error<{exc}>: No response!'
+
+        return cls(
+            username=username,
+            error=error,
+            error_code=error_data.get('error', f'status-{status_code}' if status_code is not None else None),
+            error_desc=error_data.get('error_description', error_data.get('errorMessage')),
+            status_code=status_code,
+        )
+
+    @property
+    def is_already_exists(self):
+        # HTTP 409 means "Conflict" and is returned by Keycloak when a resource already exists.
+        if self.status_code == 409:
+            return True
+
+        error_text = ' '.join(filter(None, [self.error_code, self.error_desc, self.error])).lower()
+        return POSTGRES_DUPLICATE_KEY_MARKER in error_text
 
     def __str__(self):
         return f'ImportUserError: {self.error} for {self.username}'

src/thunderbird_accounts/authentication/tests/test_api.py

@@ -221,6 +221,52 @@ def test_import_errors_propagate(self, mock_import_user: MagicMock):
         self.assertEqual(resp_data.get('type'), mock_test_type, msg=assert_fail_msg)
         self.assertEqual(resp_data.get('error'), mock_test_error, msg=assert_fail_msg)
 
+    def test_import_already_exists_error_does_not_capture_sentry_when_local_user_exists(
+        self, mock_import_user: MagicMock
+    ):
+        def create_local_user_and_raise(username, email, **_kwargs):
+            User.objects.create(username=username, email=email, recovery_email=email)
+            raise ImportUserError(
+                'Error<409>: User exists with same username',
+                username=username,
+                error_code='status-409',
+                error_desc='User exists with same username',
+                status_code=409,
+            )
+
+        mock_import_user.side_effect = create_local_user_and_raise
+
+        with patch('thunderbird_accounts.authentication.api.sentry_sdk.capture_exception') as mock_capture_exception:
+            response = self.client.post(
+                '/api/v1/auth/sign-up/',
+                self.make_sign_up_data(email=self.wait_list[0]),
+            )
+
+        assert_fail_msg = self.get_messages(response)
+        self.assertEqual(response.status_code, 400, msg=assert_fail_msg)
+        self.assertEqual(response.json()['type'], 'status-409', msg=assert_fail_msg)
+        mock_capture_exception.assert_not_called()
+
+    def test_import_already_exists_error_captures_sentry_without_local_user(self, mock_import_user: MagicMock):
+        mock_import_user.side_effect = ImportUserError(
+            'Error<409>: User exists with same username',
+            username='hello',
+            error_code='status-409',
+            error_desc='User exists with same username',
+            status_code=409,
+        )
+
+        with patch('thunderbird_accounts.authentication.api.sentry_sdk.capture_exception') as mock_capture_exception:
+            response = self.client.post(
+                '/api/v1/auth/sign-up/',
+                self.make_sign_up_data(email=self.wait_list[0]),
+            )
+
+        assert_fail_msg = self.get_messages(response)
+        self.assertEqual(response.status_code, 400, msg=assert_fail_msg)
+        self.assertEqual(response.json()['type'], 'status-409', msg=assert_fail_msg)
+        mock_capture_exception.assert_called_once()
+
 
 @override_settings(USE_ALLOW_LIST=True)
 class CanISignUpTestcase(APITestCase):

src/thunderbird_accounts/mail/views.py

@@ -489,10 +489,17 @@ def add_email_alias(request: HttpRequest):
         )
 
         if not created:
+            logging.info('Alias creation found an existing local email record for the requested address')
             return JsonResponse(
                 {'success': False, 'error': _('This email address is not available.')},
                 status=400,
             )
+    except IntegrityError:
+        logging.info('Alias creation hit a local duplicate-address race')
+        return JsonResponse(
+            {'success': False, 'error': _('This email address is not available.')},
+            status=400,
+        )
     except Exception as e:
         logging.error(f'Error creating email alias: {e}')
         return JsonResponse(