From b0dc637dbd3ea8744764557afd84399a341f7c86 Mon Sep 17 00:00:00 2001 From: neo773 <62795688+neo773@users.noreply.github.com> Date: Fri, 10 Jul 2026 23:58:34 +0530 Subject: [PATCH] Throw proper error on duplicate emailing domain (#22790) Adding an emailing domain that already exists blew up with a raw QueryFailedError and the client just saw a generic "An error occurred". The unique index on domain is global, so the workspace-scoped existence check never caught rows owned by another workspace. Now the check is unscoped and throws an EmailingDomainException mapped to CONFLICT with a proper user-facing message, in both the createEmailingDomain mutation and the email group channel flow. Also dropped the hardcoded catch-all snackbar on the new channel page so server messages actually reach the user. Review in cubic --- .../SettingsAccountsNewEmailGroupChannel.tsx | 22 ++++-------- .../hooks/useCreateEmailGroupChannel.ts | 10 +++++- .../prefer-workspace-scoped-repository.ts | 3 ++ .../emailing-domain.resolver.ts | 6 +++- .../exceptions/emailing-domain.exception.ts | 34 +++++++++++++++++++ ...ing-domain-graphql-api-exception.filter.ts | 22 ++++++++++++ .../services/emailing-domain.service.ts | 27 ++++++++++----- ...nnel-graphql-api-exception-handler.util.ts | 15 ++++++++ 8 files changed, 113 insertions(+), 26 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception.ts create mode 100644 packages/twenty-server/src/engine/core-modules/emailing-domain/filters/emailing-domain-graphql-api-exception.filter.ts diff --git a/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsNewEmailGroupChannel.tsx b/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsNewEmailGroupChannel.tsx index 742a576e98..da60c6f82e 100644 --- a/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsNewEmailGroupChannel.tsx +++ b/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsNewEmailGroupChannel.tsx @@ -10,7 +10,6 @@ import { Section } from 'twenty-ui/layout'; import { useCreateEmailGroupChannel } from '@/settings/accounts/hooks/useCreateEmailGroupChannel'; import { SaveAndCancelButtons } from '@/settings/components/SaveAndCancelButtons/SaveAndCancelButtons'; import { SettingsPageContainer } from '@/settings/components/SettingsPageContainer'; -import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; import { SettingsTextInput } from '@/ui/input/components/SettingsTextInput'; import { SettingsPageLayout } from '@/settings/components/layout/SettingsPageLayout'; import { useNavigateSettings } from '~/hooks/useNavigateSettings'; @@ -18,7 +17,6 @@ import { useNavigateSettings } from '~/hooks/useNavigateSettings'; export const SettingsAccountsNewEmailGroupChannel = () => { const { t } = useLingui(); const navigate = useNavigateSettings(); - const { enqueueErrorSnackBar } = useSnackBar(); const { createEmailGroupChannel, loading } = useCreateEmailGroupChannel(); const [handle, setHandle] = useState(''); @@ -27,22 +25,16 @@ export const SettingsAccountsNewEmailGroupChannel = () => { const canSave = isHandleValidEmail && !loading; const handleSave = useCallback(async () => { - try { - const result = await createEmailGroupChannel(handle); - const messageChannelId = - result.data?.createEmailGroupChannel.messageChannel.id; + const result = await createEmailGroupChannel(handle); + const messageChannelId = + result.data?.createEmailGroupChannel.messageChannel.id; - if (messageChannelId) { - navigate(SettingsPath.EmailGroupChannelDetail, { - messageChannelId, - }); - } - } catch { - enqueueErrorSnackBar({ - message: t`Failed to create email channel. Email channels may not be configured on this server.`, + if (messageChannelId) { + navigate(SettingsPath.EmailGroupChannelDetail, { + messageChannelId, }); } - }, [createEmailGroupChannel, handle, navigate, enqueueErrorSnackBar, t]); + }, [createEmailGroupChannel, handle, navigate]); return ( { + const { enqueueErrorSnackBar } = useSnackBar(); + const [mutate, { loading, error }] = useMutation< CreateEmailGroupChannelResult, CreateEmailGroupChannelVariables @@ -45,7 +48,12 @@ export const useCreateEmailGroupChannel = () => { }); const createEmailGroupChannel = (handle: string) => - mutate({ variables: { input: { handle } } }); + mutate({ + variables: { input: { handle } }, + onError: (mutationError) => { + enqueueErrorSnackBar({ apolloError: mutationError }); + }, + }); return { createEmailGroupChannel, loading, error }; }; diff --git a/packages/twenty-oxlint-rules/rules/prefer-workspace-scoped-repository.ts b/packages/twenty-oxlint-rules/rules/prefer-workspace-scoped-repository.ts index b293a02ab4..75c34dd359 100644 --- a/packages/twenty-oxlint-rules/rules/prefer-workspace-scoped-repository.ts +++ b/packages/twenty-oxlint-rules/rules/prefer-workspace-scoped-repository.ts @@ -50,6 +50,9 @@ const WORKSPACE_SCOPED_EXEMPTIONS = new Set([ // Only injection lives in a frozen historical upgrade-version-command // directory that CI's mutation-guard refuses to let us edit. 'DataSourceEntity', + // The domain column is globally unique across workspaces, so duplicate + // preflight checks must query cross-workspace; writes stay on the wrapper. + 'EmailingDomainEntity', ]); // Everything else must use @InjectWorkspaceScopedRepository. diff --git a/packages/twenty-server/src/engine/core-modules/emailing-domain/emailing-domain.resolver.ts b/packages/twenty-server/src/engine/core-modules/emailing-domain/emailing-domain.resolver.ts index 84b99ece92..757cbf7a6a 100644 --- a/packages/twenty-server/src/engine/core-modules/emailing-domain/emailing-domain.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/emailing-domain/emailing-domain.resolver.ts @@ -8,6 +8,7 @@ import { MetadataResolver } from 'src/engine/api/graphql/graphql-config/decorato import { CreateEmailingDomainInput } from 'src/engine/core-modules/emailing-domain/dtos/create-emailing-domain.input'; import { EmailingDomainDTO } from 'src/engine/core-modules/emailing-domain/dtos/emailing-domain.dto'; import { EmailGroupAccessGraphqlApiExceptionFilter } from 'src/engine/core-modules/emailing-domain/filters/email-group-access-graphql-api-exception.filter'; +import { EmailingDomainGraphqlApiExceptionFilter } from 'src/engine/core-modules/emailing-domain/filters/emailing-domain-graphql-api-exception.filter'; import { EmailGroupAccessService } from 'src/engine/core-modules/emailing-domain/services/email-group-access.service'; import { EmailingDomainService } from 'src/engine/core-modules/emailing-domain/services/emailing-domain.service'; import { ResolverValidationPipe } from 'src/engine/core-modules/graphql/pipes/resolver-validation.pipe'; @@ -25,7 +26,10 @@ import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; FeatureFlagGuard, SettingsPermissionGuard(PermissionFlagType.WORKSPACE), ) -@UseFilters(EmailGroupAccessGraphqlApiExceptionFilter) +@UseFilters( + EmailGroupAccessGraphqlApiExceptionFilter, + EmailingDomainGraphqlApiExceptionFilter, +) @UsePipes(ResolverValidationPipe) @MetadataResolver(() => EmailingDomainDTO) export class EmailingDomainResolver { diff --git a/packages/twenty-server/src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception.ts b/packages/twenty-server/src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception.ts new file mode 100644 index 0000000000..b2f53107c9 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception.ts @@ -0,0 +1,34 @@ +import { type MessageDescriptor } from '@lingui/core'; +import { msg } from '@lingui/core/macro'; +import { assertUnreachable } from 'twenty-shared/utils'; + +import { CustomException } from 'src/utils/custom-exception'; + +export enum EmailingDomainExceptionCode { + EMAILING_DOMAIN_ALREADY_REGISTERED = 'EMAILING_DOMAIN_ALREADY_REGISTERED', +} + +const getEmailingDomainExceptionUserFriendlyMessage = ( + code: EmailingDomainExceptionCode, +) => { + switch (code) { + case EmailingDomainExceptionCode.EMAILING_DOMAIN_ALREADY_REGISTERED: + return msg`This domain is already registered.`; + default: + assertUnreachable(code); + } +}; + +export class EmailingDomainException extends CustomException { + constructor( + message: string, + code: EmailingDomainExceptionCode, + { userFriendlyMessage }: { userFriendlyMessage?: MessageDescriptor } = {}, + ) { + super(message, code, { + userFriendlyMessage: + userFriendlyMessage ?? + getEmailingDomainExceptionUserFriendlyMessage(code), + }); + } +} diff --git a/packages/twenty-server/src/engine/core-modules/emailing-domain/filters/emailing-domain-graphql-api-exception.filter.ts b/packages/twenty-server/src/engine/core-modules/emailing-domain/filters/emailing-domain-graphql-api-exception.filter.ts new file mode 100644 index 0000000000..c5cb4e0b4b --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/emailing-domain/filters/emailing-domain-graphql-api-exception.filter.ts @@ -0,0 +1,22 @@ +import { Catch, type ExceptionFilter } from '@nestjs/common'; + +import { assertUnreachable } from 'twenty-shared/utils'; + +import { + EmailingDomainException, + EmailingDomainExceptionCode, +} from 'src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception'; +import { ConflictError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; + +@Catch(EmailingDomainException) +export class EmailingDomainGraphqlApiExceptionFilter implements ExceptionFilter { + catch(exception: EmailingDomainException) { + switch (exception.code) { + case EmailingDomainExceptionCode.EMAILING_DOMAIN_ALREADY_REGISTERED: + throw new ConflictError(exception); + default: { + assertUnreachable(exception.code); + } + } + } +} diff --git a/packages/twenty-server/src/engine/core-modules/emailing-domain/services/emailing-domain.service.ts b/packages/twenty-server/src/engine/core-modules/emailing-domain/services/emailing-domain.service.ts index dc801ee30a..2895be42ee 100644 --- a/packages/twenty-server/src/engine/core-modules/emailing-domain/services/emailing-domain.service.ts +++ b/packages/twenty-server/src/engine/core-modules/emailing-domain/services/emailing-domain.service.ts @@ -1,7 +1,10 @@ import { Injectable, Logger } from '@nestjs/common'; +import { InjectRepository } from '@nestjs/typeorm'; import { isDefined } from 'twenty-shared/utils'; +import { Repository } from 'typeorm'; + import { EmailingDomainDriverException, EmailingDomainDriverExceptionCode, @@ -9,6 +12,10 @@ import { import { EmailingDomainDriverFactory } from 'src/engine/core-modules/emailing-domain/drivers/emailing-domain-driver.factory'; import { EmailingDomainStatus } from 'src/engine/core-modules/emailing-domain/drivers/types/emailing-domain-status.type'; import { EmailingDomainEntity } from 'src/engine/core-modules/emailing-domain/emailing-domain.entity'; +import { + EmailingDomainException, + EmailingDomainExceptionCode, +} from 'src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception'; import { UnsubscribeHostnameService } from 'src/engine/core-modules/emailing-domain/services/unsubscribe-hostname.service'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { InjectWorkspaceScopedRepository } from 'src/engine/twenty-orm/workspace-scoped-repository/inject-workspace-scoped-repository.decorator'; @@ -21,6 +28,10 @@ export class EmailingDomainService { constructor( @InjectWorkspaceScopedRepository(EmailingDomainEntity) private readonly emailingDomainRepository: WorkspaceScopedRepository, + // Domain is globally unique across workspaces, so existence checks need + // an unscoped repository + @InjectRepository(EmailingDomainEntity) + private readonly globalEmailingDomainRepository: Repository, private readonly emailingDomainDriverFactory: EmailingDomainDriverFactory, private readonly unsubscribeHostnameService: UnsubscribeHostnameService, ) {} @@ -29,17 +40,15 @@ export class EmailingDomainService { domain: string, workspaceId: string, ): Promise { - const existingEmailingDomain = await this.emailingDomainRepository.findOne( - workspaceId, - { + const existingEmailingDomain = + await this.globalEmailingDomainRepository.findOne({ where: { domain }, - }, - ); + }); - if (existingEmailingDomain) { - throw new EmailingDomainDriverException( - 'Emailing domain already exists for this workspace', - EmailingDomainDriverExceptionCode.CONFIGURATION_ERROR, + if (isDefined(existingEmailingDomain)) { + throw new EmailingDomainException( + 'Emailing domain is already registered', + EmailingDomainExceptionCode.EMAILING_DOMAIN_ALREADY_REGISTERED, ); } diff --git a/packages/twenty-server/src/engine/metadata-modules/message-channel/utils/message-channel-graphql-api-exception-handler.util.ts b/packages/twenty-server/src/engine/metadata-modules/message-channel/utils/message-channel-graphql-api-exception-handler.util.ts index f6de0151b4..4ce5ac9bed 100644 --- a/packages/twenty-server/src/engine/metadata-modules/message-channel/utils/message-channel-graphql-api-exception-handler.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/message-channel/utils/message-channel-graphql-api-exception-handler.util.ts @@ -1,6 +1,11 @@ import { assertUnreachable } from 'twenty-shared/utils'; import { + EmailingDomainException, + EmailingDomainExceptionCode, +} from 'src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception'; +import { + ConflictError, ForbiddenError, InternalServerError, NotFoundError, @@ -32,6 +37,16 @@ export const messageChannelGraphqlApiExceptionHandler = (error: Error) => { } } + if (error instanceof EmailingDomainException) { + switch (error.code) { + case EmailingDomainExceptionCode.EMAILING_DOMAIN_ALREADY_REGISTERED: + throw new ConflictError(error); + default: { + return assertUnreachable(error.code); + } + } + } + if (error instanceof ConnectedAccountException) { switch (error.code) { case ConnectedAccountExceptionCode.CONNECTED_ACCOUNT_OWNERSHIP_VIOLATION: