Provider Users Watcher - Actual Bugs¶

Overview¶

This document lists actual bugs found in the Provider Users Watcher code that could cause crashes in staging. These are specific issues that would cause the code to fail, not architectural concerns.

Found Bugs¶

1. Incorrect Token Update¶

provider_user.tokens = AuthTokens(
    accessToken=access_token,
    refreshToken=refresh_token,
)
provider_user.scopes = granted_scopes
provider_user.googleAuthCode = None

Bug: The code updates the provider user object but doesn't handle the case where granted_scopes is None. This could cause a crash when trying to store None in Firestore.

2. Race Condition in Service State¶

self.previous_services[provider_user_id] = current_services.copy()

Bug: The code modifies a shared dictionary without any synchronization. If multiple updates happen simultaneously, it could lead to corrupted state.

3. Incorrect Future Cleanup¶

def unsubscribe() -> None:
    listener.unsubscribe()
    for future in watcher._futures:
        if not future.done():
            future.cancel()

Bug: The code cancels futures but doesn't remove them from the _futures list. This could lead to a memory leak as the list grows unbounded.

4. Missing Required Field Validation¶

app_user_id = doc_data.get("appUserId")
family_id = doc_data.get("familyId")
if not app_user_id:
    self.logger.error(f"No app_user_id found for provider user {provider_user_id}")
    return

Bug: The code checks for app_user_id but not for family_id, which is used later in the code. This could cause a crash when trying to create a sync progress record.

5. Incorrect Date Handling¶

end_date = datetime.now(tz=UTC)
start_date = end_date - timedelta(days=7)

Bug: The code creates datetime objects but then converts them to ISO format strings. When these are used in the email processing, they're passed as a tuple, which could cause type mismatches.

6. Incorrect Token Exchange¶

def fetch_token():
    try:
        flow.fetch_token(code=google_auth_code)
        return flow.credentials
    except Warning as w:
        self.logger.warning(f"Warning during token exchange: {str(w)}")
    except Exception as e:
        self.logger.error(f"Token exchange failed: {str(e)}")
        raise

Bug: The function catches a Warning but doesn't return anything in that case, which could lead to None being returned and causing a crash when trying to access credentials.

7. Incorrect Document Change Handling¶

if change.type == ChangeType.ADDED:
    await self.handle_added(doc, doc_data, provider_user_id)

if change.type == ChangeType.MODIFIED:
    await self.handle_modified(
        doc, doc_data, provider_user_id, current_services, previous_services
    )

Bug: The code doesn't handle ChangeType.REMOVED, which could lead to stale data in previous_services and incorrect service activation detection.

8. Incorrect Future Exception Handling¶

def _log_future_exception(fut, operation: str):
    try:
        fut.result()
    except Exception as e:
        self.logger.error(
            f"Exception in {operation}",
            exc_info=True,
            extra={"error": str(e)},
        )

Bug: The function logs the error but doesn't propagate it, which could mask critical failures and lead to inconsistent state.

9. Incorrect Service Activation Check¶

def _is_gmail_just_activated(
    self, current_services: dict, previous_services: dict
) -> bool:
    return current_services.get("gmail") is True and not previous_services.get(
        "gmail"
    )

Bug: The function doesn't handle the case where current_services or previous_services is None, which could cause a crash.

10. Incorrect Notification Creation¶

notification_doc = {
    "syncId": sync_id,
    "appUserId": app_user_id,
    "familyId": family_id,
    "providerUserId": provider_user_id,
    "type": "email",
    "status": "pending",
    "progress": 0,
    "error": None,
    "startedAt": None,
    "updatedAt": None,
    "details": {},
}

Bug: The code creates a notification document with startedAt and updatedAt as None, but these fields might be required in Firestore, causing a validation error.

Next Steps¶

These bugs should be fixed in order of severity: 1. Fix the token update to handle None values 2. Add proper handling for REMOVED document changes 3. Fix the future cleanup to prevent memory leaks 4. Add proper validation for all required fields 5. Fix the date handling to ensure consistent types 6. Fix the token exchange to handle all cases 7. Fix the service activation check to handle None values 8. Fix the notification creation to include required timestamps 9. Fix the future exception handling to propagate errors 10. Fix the race condition in service state updates