IMAP Error handling enhancement and fix revoked authentication edge case (#17133)

## Changes - Removed complex retry logic in `ImapClientProvider` by delegating to root orchestrator - Added `parseImapAuthenticationError` for handling revoked authentication credentials previously the code existed in `parse-imap-error.util` but it didn't belong there and nor was not used in `ImapClientProvider` causing revoked channels to be stuck in limbo - Removed `parseImapError` from `*-error-handler.service.ts ` - Created `isImapNetworkError` for consistency with Gmail and Microsoft - `isImapNetworkError` is called at utility level for consistency
2026-01-14 18:30:43 +05:30
parent ef8d7c18c1
commit 0412ce2cda
9 changed files with 141 additions and 190 deletions
@@ -1,4 +1,5 @@
 export enum MessageNetworkExceptionCode {
+  ECONNREFUSED = 'ECONNREFUSED',
  ECONNRESET = 'ECONNRESET',
  ENOTFOUND = 'ENOTFOUND',
  ECONNABORTED = 'ECONNABORTED',
@@ -7,6 +7,7 @@ import { CustomError, isDefined } from 'twenty-shared/utils';
 import { type ImapSmtpCaldavParams } from 'src/engine/core-modules/imap-smtp-caldav-connection/types/imap-smtp-caldav-connection.type';
 import { type ConnectedAccountWorkspaceEntity } from 'src/modules/connected-account/standard-objects/connected-account.workspace-entity';
 import { MessageImportDriverExceptionCode } from 'src/modules/messaging/message-import-manager/drivers/exceptions/message-import-driver.exception';
+import { parseImapAuthenticationError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/parse-imap-authentication-error.util';

 type ConnectedAccountIdentifier = Pick<
  ConnectedAccountWorkspaceEntity,
@@ -17,14 +18,22 @@ type ConnectedAccountIdentifier = Pick<
 export class ImapClientProvider {
  private readonly logger = new Logger(ImapClientProvider.name);

-  private static readonly RETRY_ATTEMPTS = 3;
-  private static readonly RETRY_DELAY_MS = 1000;
  private static readonly CONNECTION_TIMEOUT_MS = 30000;
+  private static readonly GREETING_TIMEOUT_MS = 16000;

  async getClient(
    connectedAccount: ConnectedAccountIdentifier,
  ): Promise<ImapFlow> {
-    return this.createConnectionWithRetry(connectedAccount);
+    try {
+      return await this.createConnection(connectedAccount);
+    } catch (error) {
+      this.logger.error(
+        `Failed to establish IMAP connection for ${connectedAccount.handle}: ${error.message}`,
+        error.stack,
+      );
+
+      throw parseImapAuthenticationError(error);
+    }
  }

  async closeClient(client: ImapFlow): Promise<void> {
@@ -36,34 +45,6 @@ export class ImapClientProvider {
    }
  }

-  private async createConnectionWithRetry(
-    connectedAccount: ConnectedAccountIdentifier,
-    attempt = 1,
-  ): Promise<ImapFlow> {
-    try {
-      return await this.createConnection(connectedAccount);
-    } catch (error) {
-      if (attempt < ImapClientProvider.RETRY_ATTEMPTS) {
-        const delay = ImapClientProvider.RETRY_DELAY_MS * attempt;
-
-        this.logger.warn(
-          `IMAP connection attempt ${attempt} failed for ${connectedAccount.handle}, retrying in ${delay}ms: ${error.message}`,
-        );
-
-        await this.delay(delay);
-
-        return this.createConnectionWithRetry(connectedAccount, attempt + 1);
-      }
-
-      this.logger.error(
-        `Failed to establish IMAP connection for ${connectedAccount.handle} after ${ImapClientProvider.RETRY_ATTEMPTS} attempts: ${error.message}`,
-        error.stack,
-      );
-
-      throw error;
-    }
-  }
-
  private async createConnection(
    connectedAccount: ConnectedAccountIdentifier,
  ): Promise<ImapFlow> {
@@ -78,46 +59,33 @@ export class ImapClientProvider {
      (connectedAccount.connectionParameters as unknown as ImapSmtpCaldavParams) ||
      {};

-    let client: ImapFlow | null = null;
-    let timeoutId: NodeJS.Timeout | null = null;
-
    if (!isDefined(connectedAccount.handle)) {
      throw new CustomError(
        'Handle is required',
        MessageImportDriverExceptionCode.CHANNEL_MISCONFIGURED,
      );
    }
+
+    const client = new ImapFlow({
+      host: connectionParameters.IMAP?.host || '',
+      port: connectionParameters.IMAP?.port || 993,
+      secure: connectionParameters.IMAP?.secure,
+      auth: {
+        user: isDefined(connectionParameters.IMAP?.username)
+          ? connectionParameters.IMAP?.username
+          : connectedAccount.handle,
+        pass: connectionParameters.IMAP?.password || '',
+      },
+      logger: false,
+      tls: {
+        rejectUnauthorized: false,
+      },
+      connectionTimeout: ImapClientProvider.CONNECTION_TIMEOUT_MS,
+      greetingTimeout: ImapClientProvider.GREETING_TIMEOUT_MS,
+    });
+
    try {
-      client = new ImapFlow({
-        host: connectionParameters.IMAP?.host || '',
-        port: connectionParameters.IMAP?.port || 993,
-        secure: connectionParameters.IMAP?.secure,
-        auth: {
-          user: isDefined(connectionParameters.IMAP?.username)
-            ? connectionParameters.IMAP?.username
-            : connectedAccount.handle,
-          pass: connectionParameters.IMAP?.password || '',
-        },
-        logger: false,
-        tls: {
-          rejectUnauthorized: false,
-        },
-      });
-
-      const connectionPromise = client.connect();
-      const timeoutPromise = new Promise<never>((_, reject) => {
-        timeoutId = setTimeout(
-          () => reject(new Error('Connection timeout')),
-          ImapClientProvider.CONNECTION_TIMEOUT_MS,
-        );
-      });
-
-      await Promise.race([connectionPromise, timeoutPromise]);
-
-      if (timeoutId) {
-        clearTimeout(timeoutId);
-        timeoutId = null;
-      }
+      await client.connect();

      this.logger.log(
        `Connected to IMAP server for ${connectedAccount.handle}`,
@@ -125,25 +93,13 @@ export class ImapClientProvider {

      return client;
    } catch (error) {
-      if (timeoutId) {
-        clearTimeout(timeoutId);
-      }
-
-      if (client) {
-        try {
-          await client.logout();
-        } catch (cleanupError) {
-          this.logger.warn(
-            `Failed to cleanup client after connection error: ${cleanupError.message}`,
-          );
-        }
+      try {
+        await client.logout();
+      } catch {
+        // Ignore cleanup errors
      }

      throw error;
    }
  }
-
-  private delay(ms: number): Promise<void> {
-    return new Promise((resolve) => setTimeout(resolve, ms));
-  }
 }
@@ -1,6 +1,5 @@
 import { Injectable, Logger } from '@nestjs/common';

-import { parseImapError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/parse-imap-error.util';
 import { parseImapMessageListFetchError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/parse-imap-message-list-fetch-error.util';

@Injectable()
@@ -12,12 +11,6 @@ export class ImapMessageListFetchErrorHandler {
      `IMAP: Error fetching message list: ${JSON.stringify(error)}`,
    );

-    const networkError = parseImapError(error, { cause: error });
-
-    if (networkError) {
-      throw networkError;
-    }
-
    throw parseImapMessageListFetchError(error, { cause: error });
  }
 }
@@ -1,6 +1,5 @@
 import { Injectable, Logger } from '@nestjs/common';

-import { parseImapError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/parse-imap-error.util';
 import { parseImapMessagesImportError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/parse-imap-messages-import-error.util';

@Injectable()
@@ -11,13 +10,6 @@ export class ImapMessagesImportErrorHandler {
    this.logger.error(
      `IMAP: Error importing message ${messageExternalId}: ${JSON.stringify(error)}`,
    );
-
-    const networkError = parseImapError(error, { cause: error });
-
-    if (networkError) {
-      throw networkError;
-    }
-
    throw parseImapMessagesImportError(error, messageExternalId, {
      cause: error,
    });
@@ -0,0 +1,54 @@
+import { isDefined } from 'twenty-shared/utils';
+
+import { MessageNetworkExceptionCode } from 'src/modules/messaging/message-import-manager/drivers/exceptions/message-network.exception';
+
+const IMAPFLOW_TIMEOUT_ERROR_CODES = [
+  'ETIMEOUT',
+  'UPGRADE_TIMEOUT',
+  'CONNECT_TIMEOUT',
+  'GREETING_TIMEOUT',
+];
+
+const IMAPFLOW_CONNECTION_ERROR_CODES = [
+  'NoConnection',
+  'EConnectionClosed',
+  'ProxyError',
+  'ClosedAfterConnectTLS',
+  'ClosedAfterConnectText',
+];
+
+const NODEJS_NETWORK_ERROR_CODES = [
+  MessageNetworkExceptionCode.ECONNREFUSED,
+  MessageNetworkExceptionCode.ECONNRESET,
+  MessageNetworkExceptionCode.ENOTFOUND,
+  MessageNetworkExceptionCode.ECONNABORTED,
+  MessageNetworkExceptionCode.ETIMEDOUT,
+  MessageNetworkExceptionCode.ERR_NETWORK,
+  MessageNetworkExceptionCode.EHOSTUNREACH,
+];
+
+export const isImapNetworkError = (error: Error): boolean => {
+  const errorWithCode = error as { code?: string };
+
+  if (!isDefined(errorWithCode.code)) {
+    return false;
+  }
+
+  if (IMAPFLOW_TIMEOUT_ERROR_CODES.includes(errorWithCode.code)) {
+    return true;
+  }
+
+  if (IMAPFLOW_CONNECTION_ERROR_CODES.includes(errorWithCode.code)) {
+    return true;
+  }
+
+  if (
+    NODEJS_NETWORK_ERROR_CODES.includes(
+      errorWithCode.code as MessageNetworkExceptionCode,
+    )
+  ) {
+    return true;
+  }
+
+  return false;
+};
@@ -0,0 +1,32 @@
+import {
+  MessageImportDriverException,
+  MessageImportDriverExceptionCode,
+} from 'src/modules/messaging/message-import-manager/drivers/exceptions/message-import-driver.exception';
+import { type ImapFlowError } from 'src/modules/messaging/message-import-manager/drivers/imap/types/imap-error.type';
+import { isImapNetworkError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/is-imap-network-error.util';
+
+export const parseImapAuthenticationError = (
+  error: ImapFlowError,
+): MessageImportDriverException => {
+  if (isImapNetworkError(error)) {
+    return new MessageImportDriverException(
+      `IMAP network error: ${error.message}`,
+      MessageImportDriverExceptionCode.TEMPORARY_ERROR,
+      { cause: error },
+    );
+  }
+
+  if (error.authenticationFailed === true) {
+    return new MessageImportDriverException(
+      `IMAP authentication error: ${error.message}`,
+      MessageImportDriverExceptionCode.INSUFFICIENT_PERMISSIONS,
+      { cause: error },
+    );
+  }
+
+  return new MessageImportDriverException(
+    `Unknown IMAP authentication error: ${error.message}`,
+    MessageImportDriverExceptionCode.UNKNOWN,
+    { cause: error },
+  );
+};
@@ -1,95 +0,0 @@
-import {
-  MessageImportDriverException,
-  MessageImportDriverExceptionCode,
-} from 'src/modules/messaging/message-import-manager/drivers/exceptions/message-import-driver.exception';
-import { MessageNetworkExceptionCode } from 'src/modules/messaging/message-import-manager/drivers/exceptions/message-network.exception';
-import { isImapFlowError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/is-imap-flow-error.util';
-
-export const parseImapError = (
-  error: Error,
-  options?: { cause?: Error },
-): MessageImportDriverException | null => {
-  if (!error) {
-    return null;
-  }
-
-  if (!isImapFlowError(error)) {
-    return null;
-  }
-
-  if (error.message.includes('Connection not available')) {
-    return new MessageImportDriverException(
-      `IMAP client not available: ${error.message}`,
-      MessageImportDriverExceptionCode.TEMPORARY_ERROR,
-      { cause: options?.cause || error },
-    );
-  }
-
-  if (error.message.includes('timeout')) {
-    return new MessageImportDriverException(
-      `IMAP connection timeout: ${error.message}`,
-      MessageNetworkExceptionCode.ETIMEDOUT,
-      { cause: options?.cause || error },
-    );
-  }
-
-  if (error.code === 'ECONNREFUSED' || error.message === 'Failed to connect') {
-    return new MessageImportDriverException(
-      `IMAP connection error: ${error.message}`,
-      MessageImportDriverExceptionCode.UNKNOWN_NETWORK_ERROR,
-      { cause: options?.cause || error },
-    );
-  }
-
-  if (error.serverResponseCode) {
-    if (error.serverResponseCode === 'AUTHENTICATIONFAILED') {
-      return new MessageImportDriverException(
-        `IMAP authentication error: ${error.responseText || error.message}`,
-        MessageImportDriverExceptionCode.INSUFFICIENT_PERMISSIONS,
-        { cause: options?.cause || error },
-      );
-    }
-
-    if (error.serverResponseCode === 'NONEXISTENT') {
-      return new MessageImportDriverException(
-        `IMAP mailbox not found: ${error.responseText || error.message}`,
-        MessageImportDriverExceptionCode.NOT_FOUND,
-        { cause: options?.cause || error },
-      );
-    }
-  }
-
-  if (error.authenticationFailed === true) {
-    return new MessageImportDriverException(
-      `IMAP authentication error: ${error.responseText || error.message}`,
-      MessageImportDriverExceptionCode.INSUFFICIENT_PERMISSIONS,
-      { cause: options?.cause || error },
-    );
-  }
-
-  if (error.message === 'Command failed') {
-    if (error.responseText) {
-      if (error.responseText.includes('Resource temporarily unavailable')) {
-        return new MessageImportDriverException(
-          `IMAP temporary error: ${error.responseText}`,
-          MessageImportDriverExceptionCode.TEMPORARY_ERROR,
-          { cause: options?.cause || error },
-        );
-      }
-
-      return new MessageImportDriverException(
-        `IMAP command failed: ${error.responseText}`,
-        MessageImportDriverExceptionCode.UNKNOWN,
-        { cause: options?.cause || error },
-      );
-    }
-
-    return new MessageImportDriverException(
-      `IMAP command failed: ${error.message}`,
-      MessageImportDriverExceptionCode.TEMPORARY_ERROR,
-      { cause: options?.cause || error },
-    );
-  }
-
-  return null;
-};
@@ -3,6 +3,7 @@ import {
  MessageImportDriverExceptionCode,
 } from 'src/modules/messaging/message-import-manager/drivers/exceptions/message-import-driver.exception';
 import { isImapFlowError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/is-imap-flow-error.util';
+import { isImapNetworkError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/is-imap-network-error.util';

 export const parseImapMessageListFetchError = (
  error: Error,
@@ -16,6 +17,14 @@ export const parseImapMessageListFetchError = (
    );
  }

+  if (isImapNetworkError(error)) {
+    return new MessageImportDriverException(
+      `IMAP network error: ${error.message}`,
+      MessageImportDriverExceptionCode.TEMPORARY_ERROR,
+      { cause: options?.cause },
+    );
+  }
+
  const errorMessage = error.message || '';

  if (!isImapFlowError(error)) {
@@ -3,6 +3,7 @@ import {
  MessageImportDriverExceptionCode,
 } from 'src/modules/messaging/message-import-manager/drivers/exceptions/message-import-driver.exception';
 import { isImapFlowError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/is-imap-flow-error.util';
+import { isImapNetworkError } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/is-imap-network-error.util';

 export const parseImapMessagesImportError = (
  error: Error,
@@ -17,6 +18,14 @@ export const parseImapMessagesImportError = (
    );
  }

+  if (isImapNetworkError(error)) {
+    return new MessageImportDriverException(
+      `IMAP network error: ${error.message}`,
+      MessageImportDriverExceptionCode.TEMPORARY_ERROR,
+      { cause: options?.cause },
+    );
+  }
+
  const errorMessage = error.message || '';

  if (!isImapFlowError(error)) {