diff --git a/src/services/error.service.test.ts b/src/services/error.service.test.ts index 11019f5659..3ad1e77f9b 100644 --- a/src/services/error.service.test.ts +++ b/src/services/error.service.test.ts @@ -234,5 +234,50 @@ describe('Error Service', () => { } }); }); + + describe('Blank or non-string server messages', () => { + test('when the response message is a list of validation texts, then it shows them', () => { + const result = errorService.castError( + createAxiosResponseError({ message: ['email must be an email'], error: 'Bad Request' }, 400), + ); + + expect(result.message).toBe('email must be an email'); + }); + + test('when the response message has several validation texts, then it joins the non-blank ones', () => { + const result = errorService.castError( + createAxiosError({ message: ['email must be an email', ' ', 1, 'password is too short'] }, 400), + ); + + expect(result.message).toBe('email must be an email, password is too short'); + expect(errorService.castError({ message: ['a', 'b'] }).message).toBe('a, b'); + }); + + test.each([[[]], [' '], [{}], [{ nested: true }]])( + 'when the response message is %j, then it falls back to the error message', + (blankMessage) => { + const result = errorService.castError(createAxiosResponseError({ message: blankMessage }, 502)); + + expect(result.message).toBe('Request failed with status code 502'); + }, + ); + + test('when the message is blank and the error field is valid, then it uses the error field', () => { + const result = errorService.castError(createAxiosError({ message: ' ', error: 'Bad Gateway' }, 502)); + + expect(result.message).toBe('Bad Gateway'); + }); + + test('when an axios error has a blank message and error, then it falls back to the axios message', () => { + const result = errorService.castError(createAxiosError({ message: [], error: ' ' }, 502)); + + expect(result.message).toBe('Request failed'); + }); + + test('when a plain object has a non-string message, then it returns an Unknown error', () => { + expect(errorService.castError({ message: [], status: 500 }).message).toBe('Unknown error'); + expect(errorService.castError({ message: ' ' }).message).toBe('Unknown error'); + }); + }); }); }); diff --git a/src/services/error.service.ts b/src/services/error.service.ts index 5db644fa24..9635ac6f27 100644 --- a/src/services/error.service.ts +++ b/src/services/error.service.ts @@ -8,6 +8,18 @@ interface ErrorWithStatus extends Error { headers?: Record; } +const toMessageText = (candidate: unknown): string => { + const texts = Array.isArray(candidate) ? candidate : [candidate]; + return texts.filter((text): text is string => typeof text === 'string' && text.trim() !== '').join(', '); +}; + +/** + * Returns the first candidate that yields readable text: a non-blank string, or an array + * (e.g. validation errors) joined by ', ' from its non-blank string items. + */ +const pickMessage = (...candidates: unknown[]): string | undefined => + candidates.map(toMessageText).find((text) => text !== ''); + const errorService = { /** * Reports an error to Sentry @@ -32,7 +44,7 @@ const errorService = { if (err instanceof AxiosResponseError) { const data = err.data as { error?: string; message?: string } | undefined; - const message = data?.message || data?.error || err.message || 'Unknown error'; + const message = pickMessage(data?.message, data?.error, err.message) ?? 'Unknown error'; const headers = err.xRequestId ? { 'x-request-id': err.xRequestId } : undefined; return new AppError(message, err.status, data?.error, headers); } @@ -45,7 +57,7 @@ const errorService = { const axiosError = err as AxiosError<{ error?: string; message?: string }>; const responseData = axiosError.response?.data; const headers = axiosError.response?.headers as Record; - const message = responseData?.message || responseData?.error || axiosError.message || 'Unknown error'; + const message = pickMessage(responseData?.message, responseData?.error, axiosError.message) ?? 'Unknown error'; return new AppError(message, axiosError.response?.status, undefined, headers); } @@ -59,7 +71,8 @@ const errorService = { } const map = err as Record; - return map.message ? new AppError(map.message as string, map.status as number) : new AppError('Unknown error'); + const message = pickMessage(map.message); + return message ? new AppError(message, map.status as number) : new AppError('Unknown error'); }, }; diff --git a/src/views/Login/components/LogIn.tsx b/src/views/Login/components/LogIn.tsx index 443a9457bd..b9820925d9 100644 --- a/src/views/Login/components/LogIn.tsx +++ b/src/views/Login/components/LogIn.tsx @@ -29,6 +29,7 @@ import TurnstileWidget, { TurnstileWidgetHandle } from 'components/TurnstileWidg import encryptedStorageService from 'services/encrypted-storage.service'; import { isAccountSetupPending } from 'services/account-setup.service'; import { AccountSetupPendingNotice } from './AccountSetupPendingNotice'; +import { getLoginErrorMessage } from '../utils/getLoginErrorMessage'; const showNotification = ({ text, isError }: { text: string; isError: boolean }) => { notificationsService.show({ @@ -162,7 +163,7 @@ export default function LogIn(): JSX.Element { return; } - setLoginError([castedError.message]); + setLoginError([getLoginErrorMessage(castedError, translate)]); setShowErrors(true); if (castedError.status === 403 && castedError.code === 'ACCOUNT_BLOCKED') { diff --git a/src/views/Login/utils/getLoginErrorMessage.test.ts b/src/views/Login/utils/getLoginErrorMessage.test.ts new file mode 100644 index 0000000000..0e0e8cab99 --- /dev/null +++ b/src/views/Login/utils/getLoginErrorMessage.test.ts @@ -0,0 +1,69 @@ +import { AppError } from '@internxt/sdk'; +import { describe, expect, test } from 'vitest'; +import { getLoginErrorMessage } from './getLoginErrorMessage'; + +const translate = (key: string) => `t:${key}`; +const CONNECTION_LOST = 't:error.connectionLostError'; +const SERVER_ERROR = 't:error.serverError'; + +describe('Login error message', () => { + test.each([ + ['Network Error', undefined, 'ERR_NETWORK'], + ['timeout of 30000ms exceeded', undefined, 'ECONNABORTED'], + ['timeout exceeded', undefined, undefined], + ['Network Error', 500, 'ERR_NETWORK'], + ])('when the failure is "%s" (status %s, code %s), then it shows the connection lost message', (m, status, code) => { + expect(getLoginErrorMessage(new AppError(m, status, code), translate)).toBe(CONNECTION_LOST); + }); + + test.each([500, 502, 503, 504, 522])( + 'when the server answers %i, then it shows the server error message', + (status) => { + expect(getLoginErrorMessage(new AppError(`Request failed with status code ${status}`, status), translate)).toBe( + SERVER_ERROR, + ); + }, + ); + + test('when the server answers 504 with a "Gateway Timeout" message, then it shows the server error message', () => { + expect(getLoginErrorMessage(new AppError('Gateway Timeout', 504), translate)).toBe(SERVER_ERROR); + }); + + test('when a 5xx carries a raw server text, then it shows the server error message', () => { + expect(getLoginErrorMessage(new AppError('Bad Gateway', 502), translate)).toBe(SERVER_ERROR); + }); + + test.each(['', ' ', '[object Object]', 'Request failed with status code 400'])( + 'when the message is "%s", then it shows the server error message', + (message) => { + expect(getLoginErrorMessage(new AppError(message, 400), translate)).toBe(SERVER_ERROR); + }, + ); + + test('when a client-side crash has no status, then it shows the server error message', () => { + const error = new AppError("Cannot read properties of undefined (reading 'sKey')"); + + expect(getLoginErrorMessage(error, translate)).toBe(SERVER_ERROR); + }); + + test.each([ + [401, 'Wrong login credentials'], + [400, 'Wrong 2-factor auth code'], + [400, 'email must be an email'], + [429, 'Too Many Requests'], + ])('when the server answers %i with a message, then it keeps the server text', (status, message) => { + expect(getLoginErrorMessage(new AppError(message, status), translate)).toBe(message); + }); + + test('when the account is blocked, then it keeps the server text', () => { + const error = new AppError('Account blocked', 403, 'ACCOUNT_BLOCKED'); + + expect(getLoginErrorMessage(error, translate)).toBe('Account blocked'); + }); + + test('when a 429 has only the default axios text, then it shows the server error message', () => { + expect(getLoginErrorMessage(new AppError('Request failed with status code 429', 429), translate)).toBe( + SERVER_ERROR, + ); + }); +}); diff --git a/src/views/Login/utils/getLoginErrorMessage.ts b/src/views/Login/utils/getLoginErrorMessage.ts new file mode 100644 index 0000000000..d366342b39 --- /dev/null +++ b/src/views/Login/utils/getLoginErrorMessage.ts @@ -0,0 +1,30 @@ +import { AppError } from '@internxt/sdk'; + +type Translate = (key: string) => string; + +const NETWORK_ERROR_CODES = new Set(['ERR_NETWORK', 'ECONNABORTED', 'ETIMEDOUT', 'ECONNRESET', 'ERR_CANCELED']); +const NETWORK_ERROR_MESSAGE = /network error|timeout|failed to fetch|load failed/i; +const AXIOS_DEFAULT_MESSAGE = /^Request failed with status code \d+$/; +const GARBLED_MESSAGE = /^\[object \w+\]$/; + +/** + * Maps a login failure to the text shown in the form. + * + * - No response or timeout (network error code, or no status plus a network-like message): connection lost. + * - A real HTTP status is never overridden by message text (a 504 "Gateway Timeout" is a server error). + * - 4xx (including 429) with a meaningful server message: that message, untouched. + * - Everything else (5xx, blank/garbled/default axios text, client crash on a malformed response): generic server error. + */ +export const getLoginErrorMessage = (error: AppError, translate: Translate): string => { + const message = error.message?.trim() ?? ''; + const hasNetworkCode = error.code !== undefined && NETWORK_ERROR_CODES.has(error.code); + const hasNoResponse = error.status === undefined; + const isNetworkFailure = hasNetworkCode || (hasNoResponse && NETWORK_ERROR_MESSAGE.test(message)); + if (isNetworkFailure) return translate('error.connectionLostError'); + + const isClientError = error.status !== undefined && error.status >= 400 && error.status < 500; + const isMeaningfulMessage = message !== '' && !AXIOS_DEFAULT_MESSAGE.test(message) && !GARBLED_MESSAGE.test(message); + if (isClientError && isMeaningfulMessage) return error.message; + + return translate('error.serverError'); +};