Skip to content

Fix message sync#16186

Merged
charlesBochet merged 2 commits intomainfrom
ej/fix-mail
Nov 28, 2025
Merged

Fix message sync#16186
charlesBochet merged 2 commits intomainfrom
ej/fix-mail

Conversation

@etiennejouan etiennejouan changed the title fix Fix message sync Nov 28, 2025
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Nov 28, 2025

Greptile Overview

Greptile Summary

Removed early validation for IMAP username and handle fields after PR #16144 made these fields nullable. The change now allows null/undefined values to fall back to empty strings when authenticating with IMAP servers.

Key Changes:

  • Removed MessageImportDriverException with CHANNEL_MISCONFIGURED error code for missing username/handle
  • Removed CustomError import (no longer used)
  • Added || '' fallback operators for potentially null username and handle values

Concerns:

  • Empty string credentials will be sent to IMAP servers when username/handle are null, causing authentication failures with misleading INSUFFICIENT_PERMISSIONS errors instead of the more specific CHANNEL_MISCONFIGURED error
  • This makes debugging harder as the root cause (missing configuration) is obscured by a generic auth failure
  • The Sentry issue this PR addresses may be related to null values being introduced by PR Update workspace entities to make all TEXT nullable #16144, but the solution of allowing empty strings may create a worse user experience than proper validation

Confidence Score: 3/5

  • This PR prevents crashes but introduces misleading error messages that will make debugging harder
  • The change technically resolves the immediate Sentry issue by not throwing errors for null username/handle, but it replaces early, clear error detection with deferred, misleading errors. When IMAP authentication fails with empty credentials, users will see INSUFFICIENT_PERMISSIONS instead of CHANNEL_MISCONFIGURED, making it harder to identify that the account is misconfigured. A better solution would validate that username/handle are non-empty strings before attempting connection.
  • The imap-client.provider.ts file needs attention - the empty string fallback should be replaced with proper validation

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/providers/imap-client.provider.ts 3/5 Removed validation checks for username and handle, now allowing empty strings to be passed to IMAP auth. This changes error handling from early CHANNEL_MISCONFIGURED to later INSUFFICIENT_PERMISSIONS failures.

Sequence Diagram

sequenceDiagram
    participant Client as IMAP Client Provider
    participant DB as Database
    participant IMAP as IMAP Server
    
    Note over Client,IMAP: Before (PR #16144)
    Client->>DB: Get connected account
    DB-->>Client: Returns account with handle and username
    Client->>Client: Validate username exists
    Client->>Client: Validate handle exists
    alt Validation fails
        Client-->>Client: Throw CHANNEL_MISCONFIGURED error
    end
    Client->>IMAP: Connect with credentials
    IMAP-->>Client: Auth success/failure
    
    Note over Client,IMAP: After (This PR)
    Client->>DB: Get connected account
    DB-->>Client: Returns account (handle/username may be null)
    Client->>Client: Skip validation
    Client->>Client: Use empty string fallback for null values
    Client->>IMAP: Connect with credentials (possibly empty strings)
    alt Auth fails with empty credentials
        IMAP-->>Client: Authentication failed
        Client-->>Client: Throw INSUFFICIENT_PERMISSIONS error
    else Auth succeeds
        IMAP-->>Client: Connection established
    end
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/providers/imap-client.provider.ts, line 91-94 (link)

    logic: Empty string fallback will cause misleading error messages. When username or handle are null/undefined, this passes empty strings to IMAP auth, which will fail with INSUFFICIENT_PERMISSIONS instead of the more accurate CHANNEL_MISCONFIGURED error code. Consider validating these fields are non-empty strings before attempting connection.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 28, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:58359

This environment will automatically shut down when the PR is closed or after 5 hours.

@charlesBochet charlesBochet merged commit cd699cb into main Nov 28, 2025
53 checks passed
@charlesBochet charlesBochet deleted the ej/fix-mail branch November 28, 2025 16:49
@twenty-eng-sync
Copy link
Copy Markdown

Hey @etiennejouan! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants