From 481f9bf2eb109dcd14ec3f264104a66d32ee32df Mon Sep 17 00:00:00 2001 From: Ramon Candel Date: Thu, 5 Feb 2026 12:43:16 +0100 Subject: [PATCH 1/2] Enhance error handling and notifications for upload and folder content loading errors --- src/components/modals/AddModal/index.tsx | 6 +- src/contexts/Drive/Drive.context.tsx | 14 + .../DriveFolderScreen/DriveFolderScreen.tsx | 48 +- src/services/ErrorService.spec.ts | 472 ++++++++++++++++++ src/services/ErrorService.ts | 103 +++- 5 files changed, 620 insertions(+), 23 deletions(-) create mode 100644 src/services/ErrorService.spec.ts diff --git a/src/components/modals/AddModal/index.tsx b/src/components/modals/AddModal/index.tsx index 2acf91e6d..83109c16d 100644 --- a/src/components/modals/AddModal/index.tsx +++ b/src/components/modals/AddModal/index.tsx @@ -543,9 +543,10 @@ function AddModal(): JSX.Element { }) .catch((err) => { logger.error('Error on handleUploadFromCameraRoll function:', JSON.stringify(err)); + const error = errorService.castError(err, 'upload'); notificationsService.show({ type: NotificationType.Error, - text1: strings.formatString(strings.errors.uploadFile, err.message) as string, + text1: error.message, }); }) .finally(() => { @@ -638,9 +639,10 @@ function AddModal(): JSX.Element { }) .catch((err) => { logger.error('Error on handleUploadFromCameraRoll (Android):', JSON.stringify(err)); + const error = errorService.castError(err, 'upload'); notificationsService.show({ type: NotificationType.Error, - text1: strings.formatString(strings.errors.uploadFile, err.message) as string, + text1: error.message, }); }) .finally(() => { diff --git a/src/contexts/Drive/Drive.context.tsx b/src/contexts/Drive/Drive.context.tsx index eb4f90b41..2eaa80940 100644 --- a/src/contexts/Drive/Drive.context.tsx +++ b/src/contexts/Drive/Drive.context.tsx @@ -8,7 +8,9 @@ import React, { useEffect, useRef, useState } from 'react'; import appService from '@internxt-mobile/services/AppService'; import errorService from '@internxt-mobile/services/ErrorService'; +import notificationsService from '@internxt-mobile/services/NotificationsService'; import { AppStateStatus, NativeEventSubscription } from 'react-native'; +import { NotificationType } from '@internxt-mobile/types/index'; import { driveFolderService } from '@internxt-mobile/services/drive/folder'; import { mapFileWithIsFolder, mapFolderWithIsFolder } from 'src/helpers/driveItemMappers'; @@ -87,7 +89,13 @@ export const DriveContextProvider: React.FC = ({ chil const handleAppStateChange = (state: AppStateStatus) => { if (state === 'active' && currentFolderId.current) { loadFolderContent(currentFolderId.current, { pullFrom: ['network'], resetPagination: true }).catch((error) => { + // TODO: Refactor to custom hook (useDriveWithNotifications) to separate notification concerns from context errorService.reportError(error); + const err = errorService.castError(error, 'content'); + notificationsService.show({ + type: NotificationType.Error, + text1: err.message, + }); }); } }; @@ -115,7 +123,13 @@ export const DriveContextProvider: React.FC = ({ chil setDriveFoldersTree({ [rootFolderId]: ROOT_FOLDER_NODE }); loadFolderContent(rootFolderId, { pullFrom: ['network'], resetPagination: true, focusFolder: true }).catch( (err) => { + // TODO: Refactor to custom hook (useDriveWithNotifications) to separate notification concerns from context errorService.reportError(err); + const error = errorService.castError(err, 'content'); + notificationsService.show({ + type: NotificationType.Error, + text1: error.message, + }); }, ); }, [rootFolderId]); diff --git a/src/screens/drive/DriveFolderScreen/DriveFolderScreen.tsx b/src/screens/drive/DriveFolderScreen/DriveFolderScreen.tsx index b4f01cd36..f1cf903be 100644 --- a/src/screens/drive/DriveFolderScreen/DriveFolderScreen.tsx +++ b/src/screens/drive/DriveFolderScreen/DriveFolderScreen.tsx @@ -99,6 +99,12 @@ export function DriveFolderScreen({ navigation }: DriveScreenProps<'DriveFolder' }) .catch((error) => { logger.error('Error loading folder content in DriveFolderScreen:', error); + errorService.reportError(error); + const err = errorService.castError(error, 'content'); + notificationsService.show({ + type: NotificationType.Error, + text1: err.message, + }); }); } }, [folderUuid]); @@ -109,7 +115,14 @@ export function DriveFolderScreen({ navigation }: DriveScreenProps<'DriveFolder' if (parentUuid) { driveCtx .loadFolderContent(parentUuid, { pullFrom: ['network'], resetPagination: false, focusFolder: true }) - .catch(errorService.reportError); + .catch((error) => { + errorService.reportError(error); + const err = errorService.castError(error, 'content'); + notificationsService.show({ + type: NotificationType.Error, + text1: err.message, + }); + }); } }; @@ -193,7 +206,14 @@ export function DriveFolderScreen({ navigation }: DriveScreenProps<'DriveFolder' ); handleOnFilePress(driveItem); } else if (driveItem.data.uuid) { - driveCtx.loadFolderContent(driveItem.data.uuid, { focusFolder: true, resetPagination: true }); + driveCtx.loadFolderContent(driveItem.data.uuid, { focusFolder: true, resetPagination: true }).catch((error) => { + errorService.reportError(error); + const err = errorService.castError(error, 'content'); + notificationsService.show({ + type: NotificationType.Error, + text1: err.message, + }); + }); // Navigate to the folder, this is the minimal data navigation.push('DriveFolder', { @@ -277,6 +297,11 @@ export function DriveFolderScreen({ navigation }: DriveScreenProps<'DriveFolder' }); } catch (error) { errorService.reportError(error); + const err = errorService.castError(error, 'content'); + notificationsService.show({ + type: NotificationType.Error, + text1: err.message, + }); } finally { setLoadingMore(false); } @@ -296,11 +321,20 @@ export function DriveFolderScreen({ navigation }: DriveScreenProps<'DriveFolder' }, [driveSortedItems, searchValue]); async function handleRefresh() { - await driveCtx.loadFolderContent(folderUuid, { - focusFolder: true, - pullFrom: ['network'], - resetPagination: true, - }); + try { + await driveCtx.loadFolderContent(folderUuid, { + focusFolder: true, + pullFrom: ['network'], + resetPagination: true, + }); + } catch (error) { + errorService.reportError(error); + const err = errorService.castError(error, 'content'); + notificationsService.show({ + type: NotificationType.Error, + text1: err.message, + }); + } } return ( diff --git a/src/services/ErrorService.spec.ts b/src/services/ErrorService.spec.ts new file mode 100644 index 000000000..a8bbd04e1 --- /dev/null +++ b/src/services/ErrorService.spec.ts @@ -0,0 +1,472 @@ +import strings from '../../assets/lang/strings'; +import AppError from '../types'; +import { HTTP_TOO_MANY_REQUESTS } from './common'; +import errorService from './ErrorService'; + +describe('ErrorService', () => { + describe('castError', () => { + describe('extractStatus - multiple locations', () => { + it('when status is directly in error.status, extracts it correctly', () => { + const error = { status: 404, message: 'Not found' }; + const result = errorService.castError(error); + + expect(result).toBeInstanceOf(AppError); + expect(result.status).toBe(404); + expect(result.message).toBe('Not found'); + }); + + it('when status is in error.response.status (Axios format), extracts it correctly', () => { + const error = { + response: { + status: 500, + data: { message: 'Internal server error' }, + }, + }; + const result = errorService.castError(error); + + expect(result).toBeInstanceOf(AppError); + expect(result.status).toBe(500); + expect(result.message).toBe('Internal server error'); + }); + + it('when status is in both places, prioritizes error.status', () => { + const error = { + status: 403, + message: 'Forbidden', + response: { + status: 404, + data: { message: 'Not found' }, + }, + }; + const result = errorService.castError(error); + + expect(result.status).toBe(403); + expect(result.message).toBe('Forbidden'); + }); + + it('when there is no status in any location, returns generic error', () => { + const error = { message: 'Network error' }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when status is a string number in error.status, parses and extracts it correctly', () => { + const error = { status: '404', message: 'Not found' }; + const result = errorService.castError(error); + + expect(result.status).toBe(404); + expect(result.message).toBe('Not found'); + }); + + it('when status is a string number in error.response.status, parses and extracts it correctly', () => { + const error = { + response: { + status: '500', + data: { message: 'Internal server error' }, + }, + }; + const result = errorService.castError(error); + + expect(result.status).toBe(500); + expect(result.message).toBe('Internal server error'); + }); + + it('when status is a string with leading/trailing spaces, parses it correctly', () => { + const error = { status: ' 429 ', message: 'Rate limit exceeded' }; + const result = errorService.castError(error, 'upload'); + + expect(result.status).toBe(429); + expect(result.message).toBe(strings.errors.rateLimitUpload); + }); + + it('when status is an invalid string (non-numeric), ignores it and returns generic error', () => { + const error = { status: 'not-a-number', message: 'Some error' }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when status is out of valid HTTP range (e.g., 999), ignores it and returns generic error', () => { + const error = { status: 999, message: 'Invalid status' }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when status is below valid HTTP range (e.g., 99), ignores it and returns generic error', () => { + const error = { status: 99, message: 'Invalid status' }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when string status is at lower edge of error range (400), parses it correctly', () => { + const error = { status: '400', message: 'Bad request' }; + const result = errorService.castError(error); + + expect(result.status).toBe(400); + expect(result.message).toBe('Bad request'); + }); + + it('when string status is at upper edge of error range (599), parses it correctly', () => { + const error = { status: '599', message: 'Network error' }; + const result = errorService.castError(error); + + expect(result.status).toBe(599); + expect(result.message).toBe('Network error'); + }); + + it('when string status is informational (100-199), ignores it as not an error', () => { + const error = { status: '100', message: 'Continue' }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when string status is successful (200-299), ignores it as not an error', () => { + const error = { status: '200', message: 'OK' }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when status is a float string, parses only the integer part', () => { + const error = { status: '404.5', message: 'Not found' }; + const result = errorService.castError(error); + + expect(result.status).toBe(404); + expect(result.message).toBe('Not found'); + }); + }); + + describe('extractMessage - multiple locations', () => { + it('when message is directly in error.message, extracts it correctly', () => { + const error = { status: 400, message: 'Bad request' }; + const result = errorService.castError(error); + + expect(result.message).toBe('Bad request'); + }); + + it('when message is in error.response.data.message (API format), extracts it correctly', () => { + const error = { + response: { + status: 401, + data: { message: 'Unauthorized access' }, + }, + }; + const result = errorService.castError(error); + + expect(result.message).toBe('Unauthorized access'); + }); + + it('when message is in both places, prioritizes error.message', () => { + const error = { + message: 'Direct message', + response: { + status: 400, + data: { message: 'API message' }, + }, + }; + const result = errorService.castError(error); + + expect(result.message).toBe('Direct message'); + }); + + it('when message is an empty string, ignores it and searches in response.data.message', () => { + const error = { + message: ' ', + response: { + status: 400, + data: { message: 'Validation error' }, + }, + }; + const result = errorService.castError(error); + + expect(result.message).toBe('Validation error'); + }); + + it('when there is no message anywhere but has status, returns generic error', () => { + const error = { + response: { + status: 500, + data: {}, + }, + }; + const result = errorService.castError(error); + + expect(result.message).toBe(strings.errors.genericError); + }); + }); + + describe('rate limit (429) - detection and contextual messages', () => { + it('when receives 429 in error.status without context, returns generic rate limit message', () => { + const error = { + status: HTTP_TOO_MANY_REQUESTS, + message: 'Rate limit exceeded', + }; + const result = errorService.castError(error); + + expect(result.status).toBe(HTTP_TOO_MANY_REQUESTS); + expect(result.message).toBe(strings.errors.rateLimitReached); + }); + + it('when receives 429 in error.response.status without context, returns generic rate limit message', () => { + const error = { + response: { + status: HTTP_TOO_MANY_REQUESTS, + data: { message: 'Too many requests' }, + }, + }; + const result = errorService.castError(error); + + expect(result.status).toBe(HTTP_TOO_MANY_REQUESTS); + expect(result.message).toBe(strings.errors.rateLimitReached); + }); + + it('when receives 429 with "upload" context, returns specific upload message', () => { + const error = { + status: HTTP_TOO_MANY_REQUESTS, + message: 'Rate limit exceeded', + }; + const result = errorService.castError(error, 'upload'); + + expect(result.status).toBe(HTTP_TOO_MANY_REQUESTS); + expect(result.message).toBe(strings.errors.rateLimitUpload); + }); + + it('when receives 429 with "download" context, returns specific download message', () => { + const error = { + status: HTTP_TOO_MANY_REQUESTS, + message: 'Rate limit exceeded', + }; + const result = errorService.castError(error, 'download'); + + expect(result.status).toBe(HTTP_TOO_MANY_REQUESTS); + expect(result.message).toBe(strings.errors.rateLimitDownload); + }); + + it('when receives 429 with "content" context, returns specific content message', () => { + const error = { + status: HTTP_TOO_MANY_REQUESTS, + message: 'Rate limit exceeded', + }; + const result = errorService.castError(error, 'content'); + + expect(result.status).toBe(HTTP_TOO_MANY_REQUESTS); + expect(result.message).toBe(strings.errors.rateLimitContent); + }); + + it('when receives 429 from Axios format with "upload" context, returns correct message', () => { + const error = { + response: { + status: HTTP_TOO_MANY_REQUESTS, + headers: { + 'x-internxt-ratelimit-limit': '100', + 'x-internxt-ratelimit-remaining': '0', + 'x-internxt-ratelimit-reset': String(Date.now() + 60000), + }, + data: { message: 'Rate limit exceeded' }, + }, + }; + const result = errorService.castError(error, 'upload'); + + expect(result.status).toBe(HTTP_TOO_MANY_REQUESTS); + expect(result.message).toBe(strings.errors.rateLimitUpload); + }); + }); + + describe('server errors (4xx/5xx) - different formats', () => { + it('when receives error 400 with message, processes it correctly', () => { + const error = { + status: 400, + message: 'Invalid request parameters', + }; + const result = errorService.castError(error); + + expect(result.status).toBe(400); + expect(result.message).toBe('Invalid request parameters'); + }); + + it('when receives error 401 from API, processes it correctly', () => { + const error = { + response: { + status: 401, + data: { message: 'Authentication required' }, + }, + }; + const result = errorService.castError(error); + + expect(result.status).toBe(401); + expect(result.message).toBe('Authentication required'); + }); + + it('when receives error 403 with Spanish message, preserves it', () => { + const error = { + status: 403, + message: 'No tienes permisos para acceder a este recurso', + }; + const result = errorService.castError(error); + + expect(result.status).toBe(403); + expect(result.message).toBe('No tienes permisos para acceder a este recurso'); + }); + + it('when receives error 404 without message, returns generic error', () => { + const error = { + status: 404, + message: '', + }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when receives error 500 from server, processes it correctly', () => { + const error = { + response: { + status: 500, + data: { message: 'Internal server error' }, + }, + }; + const result = errorService.castError(error); + + expect(result.status).toBe(500); + expect(result.message).toBe('Internal server error'); + }); + + it('when receives error 503 (service unavailable), processes it correctly', () => { + const error = { + status: 503, + message: 'Service temporarily unavailable', + }; + const result = errorService.castError(error); + + expect(result.status).toBe(503); + expect(result.message).toBe('Service temporarily unavailable'); + }); + }); + + describe('edge cases and unexpected errors', () => { + it('when receives null, returns generic error', () => { + const result = errorService.castError(null); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when receives undefined, returns generic error', () => { + const result = errorService.castError(undefined); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when receives a string, returns generic error', () => { + const result = errorService.castError('Something went wrong'); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when receives a number, returns generic error', () => { + const result = errorService.castError(404); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when receives an empty object, returns generic error', () => { + const result = errorService.castError({}); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when status is not 4xx/5xx but has message, returns generic error', () => { + const error = { + status: 200, + message: 'Success but treated as error', + }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when status is 399 (out of range 400-599), returns generic error', () => { + const error = { + status: 399, + message: 'Not a valid error status', + }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when status is 600 (out of range 400-599), returns generic error', () => { + const error = { + status: 600, + message: 'Not a valid error status', + }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + + it('when status is valid but message has only spaces, returns generic error', () => { + const error = { + status: 500, + message: ' ', + response: { + data: {}, + }, + }; + const result = errorService.castError(error); + + expect(result.status).toBeUndefined(); + expect(result.message).toBe(strings.errors.genericError); + }); + }); + + describe('extraction priority verification', () => { + it('when 429 in error.status but different status in response, uses error.status', () => { + const error = { + status: HTTP_TOO_MANY_REQUESTS, + message: 'Rate limit exceeded', + response: { + status: 500, + data: { message: 'Server error' }, + }, + }; + const result = errorService.castError(error, 'upload'); + + expect(result.status).toBe(HTTP_TOO_MANY_REQUESTS); + expect(result.message).toBe(strings.errors.rateLimitUpload); + }); + + it('when direct message and message in response, uses direct message', () => { + const error = { + status: 400, + message: 'Direct error message', + response: { + status: 400, + data: { message: 'API error message' }, + }, + }; + const result = errorService.castError(error); + + expect(result.message).toBe('Direct error message'); + }); + }); + }); +}); diff --git a/src/services/ErrorService.ts b/src/services/ErrorService.ts index 0fb8c18b7..aea118121 100644 --- a/src/services/ErrorService.ts +++ b/src/services/ErrorService.ts @@ -23,6 +23,9 @@ export interface ErrorContext extends GlobalErrorContext { tags: { [tagName: string]: string }; extra?: Record; } + +export type CastErrorContext = 'upload' | 'download' | 'content'; + class ErrorService { private logger = new SentryLogger(); public setGlobalErrorContext(globalContext: Partial) { @@ -32,33 +35,105 @@ class ErrorService { // }); } - public castError(err: unknown, context?: 'upload' | 'download' | 'content'): AppError { + /** + * Converts errors from different formats to a consistent and user-friendly AppError. + * + * @param err - Error in any format (Axios, API, JavaScript, etc.) + * @param context - Optional context for specific rate limit messages + * + * @returns AppError with user-friendly message and status code (if available) + */ + public castError(err: unknown, context?: CastErrorContext): AppError { if (err && typeof err === 'object') { const map = err as Record; + const status = this.extractStatus(map); + const message = this.extractMessage(map); + + if (status === HTTP_TOO_MANY_REQUESTS) { + const rateLimitMessage = this.getRateLimitMessage(context); + return new AppError(rateLimitMessage, status); + } + const isServerReturnedError = - typeof map.message === 'string' && - map.message.trim().length > 0 && - typeof map.status === 'number' && - map.status >= 400 && - map.status < 600; + typeof message === 'string' && + message.trim().length > 0 && + typeof status === 'number' && + status >= 400 && + status < 600; if (isServerReturnedError) { - const status = map.status as number; + return new AppError(message, status); + } + } - if (status === HTTP_TOO_MANY_REQUESTS) { - const message = this.getRateLimitMessage(context); - return new AppError(message, status); - } + return new AppError(strings.errors.genericError); + } + + /** + * Extract status from error object, checking multiple possible locations: + * - error.status (direct) + * - error.response.status (axios format) + */ + private extractStatus(err: Record): number | undefined { + const directStatus = this.parseStatus(err.status); + if (directStatus !== undefined) { + return directStatus; + } - return new AppError(map.message as string, status); + const response = err.response as Record | undefined; + if (response) { + const responseStatus = this.parseStatus(response.status); + if (responseStatus !== undefined) { + return responseStatus; } } - return new AppError(strings.errors.genericError); + return undefined; + } + + /** + * Parse a status value that can be either a number or a string. + * Returns undefined if the value cannot be parsed as a valid HTTP status code. + */ + private parseStatus(value: unknown): number | undefined { + if (typeof value === 'number') { + return this.isValidHttpStatus(value) ? value : undefined; + } + + if (typeof value === 'string') { + const parsed = parseInt(value, 10); + return !Number.isNaN(parsed) && this.isValidHttpStatus(parsed) ? parsed : undefined; + } + + return undefined; + } + + /** + * Check if a number is a valid HTTP status code (100-599) + */ + private isValidHttpStatus(status: number): boolean { + return status >= 100 && status < 600; + } + + /** + * Extract message from error object, checking multiple possible locations + */ + private extractMessage(err: Record): string { + if (typeof err.message === 'string' && err.message.trim().length > 0) { + return err.message; + } + + const response = err.response as Record | undefined; + const responseData = response?.data as Record | undefined; + if (responseData && typeof responseData.message === 'string') { + return responseData.message; + } + + return ''; } - private getRateLimitMessage(context?: 'upload' | 'download' | 'content'): string { + private getRateLimitMessage(context?: CastErrorContext): string { switch (context) { case 'upload': return strings.errors.rateLimitUpload; From 5c27ac228a5134fda55f53a7386562640edfd56d Mon Sep 17 00:00:00 2001 From: Ramon Candel Date: Thu, 5 Feb 2026 12:46:51 +0100 Subject: [PATCH 2/2] Updated node version in unit tests action --- .github/workflows/unit-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 872adc65f..a6ac0b0a7 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -9,7 +9,7 @@ jobs: strategy: matrix: - node-version: [18.20.4] + node-version: [20] fail-fast: true steps: