Handle zip imports regression

src/hooks/tests/use-import-export.test.tsx

@@ -225,8 +225,32 @@ describe( 'useImportExport hook', () => {
 		expect( useSiteDetails().startServer ).toHaveBeenCalledWith( SITE_ID );
 	} );
 
-	it( 'shows error message when import fails', async () => {
-		( getIpcApi().importSite as jest.Mock ).mockRejectedValue( 'error' );
+	it( 'shows error message when import fails with absolute path error', async () => {
+		( getIpcApi().importSite as jest.Mock ).mockRejectedValue(
+			new Error( 'Error: absolute path: /' )
+		);
+
+		const { result } = renderHook( () => useImportExport(), { wrapper } );
+		const file = { path: 'backup.zip', type: 'application/zip' };
+		await act( () => result.current.importFile( file, selectedSite ) );
+
+		expect( result.current.exportState ).toEqual( {} );
+		expect( getIpcApi().importSite ).toHaveBeenCalledWith( {
+			id: SITE_ID,
+			backupFile: {
+				type: 'application/zip',
+				path: 'backup.zip',
+			},
+		} );
+		expect( getIpcApi().showErrorMessageBox ).toHaveBeenCalledWith( {
+			title: 'Failed importing site',
+			message:
+				'The ZIP archive is invalid. Try to unpack and pack it again. If this problem persists, please contact support.',
+		} );
+	} );
+
+	it( 'shows error message when import fails with other error', async () => {
+		( getIpcApi().importSite as jest.Mock ).mockRejectedValue( new Error( 'generic error' ) );
 
 		const { result } = renderHook( () => useImportExport(), { wrapper } );
 		const file = { path: 'backup.zip', type: 'application/zip' };
@@ -244,7 +268,7 @@ describe( 'useImportExport hook', () => {
 			title: 'Failed importing site',
 			message:
 				'An error occurred while importing the site. Verify the file is a valid Jetpack backup, Local, Playground, .wpress or .sql database file and try again. If this problem persists, please contact support.',
-			error: 'error',
+			error: new Error( 'generic error' ),
 			showOpenLogs: true,
 		} );
 	} );

src/hooks/use-import-export.tsx

@@ -84,15 +84,24 @@ export const ImportExportProvider = ( { children }: { children: React.ReactNode
 				},
 			} ) );
 
-			const handleImportError = async ( error?: unknown ) => {
-				await getIpcApi().showErrorMessageBox( {
-					title: __( 'Failed importing site' ),
-					message: __(
-						'An error occurred while importing the site. Verify the file is a valid Jetpack backup, Local, Playground, .wpress or .sql database file and try again. If this problem persists, please contact support.'
-					),
-					error,
-					showOpenLogs: true,
-				} );
+			const handleImportError = async ( error: unknown ) => {
+				if ( error instanceof Error && error.message.includes( 'Error: absolute path: /' ) ) {
+					await getIpcApi().showErrorMessageBox( {
+						title: __( 'Failed importing site' ),
+						message: __(
+							'The ZIP archive is invalid. Try to unpack and pack it again. If this problem persists, please contact support.'
+						),
+					} );
+				} else {
+					await getIpcApi().showErrorMessageBox( {
+						title: __( 'Failed importing site' ),
+						message: __(
+							'An error occurred while importing the site. Verify the file is a valid Jetpack backup, Local, Playground, .wpress or .sql database file and try again. If this problem persists, please contact support.'
+						),
+						error,
+						showOpenLogs: true,
+					} );
+				}
 				setImportState( ( { [ selectedSite.id ]: currentProgress, ...rest } ) => ( {
 					...rest,
 				} ) );
@@ -112,11 +121,6 @@ export const ImportExportProvider = ( { children }: { children: React.ReactNode
 					backupFile,
 				} );
 
-				if ( ! importedSite ) {
-					await handleImportError();
-					return;
-				}
-
 				await updateSite( importedSite );
 
 				if ( showImportNotification ) {

src/ipc-handlers.ts

@@ -103,7 +103,7 @@ export async function getInstalledApps( _event: IpcMainInvokeEvent ): Promise< I
 export async function importSite(
 	event: IpcMainInvokeEvent,
 	{ id, backupFile }: { id: string; backupFile: BackupArchiveInfo }
-): Promise< SiteDetails | undefined > {
+): Promise< SiteDetails > {
 	const site = SiteServer.get( id );
 	if ( ! site ) {
 		throw new Error( 'Site not found.' );
@@ -116,9 +116,6 @@ export async function importSite(
 			}
 		};
 		const result = await importBackup( backupFile, site.details, onEvent, defaultImporterOptions );
-		if ( ! result ) {
-			return;
-		}
 		if ( result?.meta?.phpVersion ) {
 			site.details.phpVersion = result.meta.phpVersion;
 		}

src/lib/import-export/import/import-manager.ts

@@ -50,22 +50,23 @@ export async function importBackup(
 	site: SiteDetails,
 	onEvent: ( data: ImportExportEventData ) => void,
 	options: ImporterOption[]
-): Promise< ImporterResult | undefined > {
+): Promise< ImporterResult > {
+	const backupHandler = BackupHandlerFactory.create( backupFile );
+	if ( ! backupHandler ) {
+		throw new Error( 'No suitable backup handler found for the provided backup file' );
+	}
+
+	const fileList = await backupHandler.listFiles( backupFile );
+	const importer = selectImporter( fileList, '', onEvent, options );
+
+	if ( ! importer ) {
+		throw new Error( 'No suitable importer found for the provided backup contents' );
+	}
+
 	const extractionDirectory = await fsPromises.mkdtemp( path.join( os.tmpdir(), 'studio_backup' ) );
 	let removeBackupListeners;
 	let removeImportListeners;
 	try {
-		const backupHandler = BackupHandlerFactory.create( backupFile );
-		if ( ! backupHandler ) {
-			return;
-		}
-		const fileList = await backupHandler.listFiles( backupFile );
-		const importer = selectImporter( fileList, extractionDirectory, onEvent, options );
-
-		if ( ! importer ) {
-			return;
-		}
-
 		removeBackupListeners = handleEvents( backupHandler, onEvent, BackupExtractEvents );
 		removeImportListeners = handleEvents( importer, onEvent, ImporterEvents );
 		await backupHandler.extractFiles( backupFile, extractionDirectory );

src/lib/import-export/tests/import/import-manager.test.ts

@@ -126,7 +126,7 @@ describe( 'importManager', () => {
 			} );
 		} );
 
-		it( 'should return false if no suitable importer is found', async () => {
+		it( 'should throw error if no suitable importer is found', async () => {
 			const mockValidator: Validator = {
 				canHandle: jest.fn().mockReturnValue( false ),
 				parseBackupContents: jest.fn(),
@@ -137,18 +137,28 @@ describe( 'importManager', () => {
 			};
 			( BackupHandlerFactory.create as jest.Mock ).mockReturnValue( mockBackupHandler );
 
-			const result = await importBackup( mockFile, mockSite, jest.fn(), [
-				{
-					validator: mockValidator,
-					importer: jest.fn(),
-				},
-			] );
+			await expect(
+				importBackup( mockFile, mockSite, jest.fn(), [
+					{
+						validator: mockValidator,
+						importer: jest.fn(),
+					},
+				] )
+			).rejects.toThrow( 'No suitable importer found for the provided backup contents' );
+
+			expect( fsPromises.mkdtemp ).not.toHaveBeenCalled();
+			expect( fsPromises.rm ).not.toHaveBeenCalled();
+		} );
 
-			expect( result ).toBeFalsy();
-			expect( fsPromises.mkdtemp ).toHaveBeenCalledWith( '/tmp/studio_backup' );
-			expect( fsPromises.rm ).toHaveBeenCalledWith( mockExtractDir, {
-				recursive: true,
-			} );
+		it( 'should throw error if no suitable backup handler is found', async () => {
+			( BackupHandlerFactory.create as jest.Mock ).mockReturnValue( null );
+
+			await expect( importBackup( mockFile, mockSite, jest.fn(), [] ) ).rejects.toThrow(
+				'No suitable backup handler found for the provided backup file'
+			);
+
+			expect( fsPromises.mkdtemp ).not.toHaveBeenCalled();
+			expect( fsPromises.rm ).not.toHaveBeenCalled();
 		} );
 	} );
 } );

src/tests/ipc-handlers.test.ts

@@ -4,8 +4,11 @@
 import { shell, IpcMainInvokeEvent } from 'electron';
 import fs from 'fs';
 import { normalize } from 'path';
-import { createSite, startServer, isFullscreen } from '../ipc-handlers';
+import * as Sentry from '@sentry/electron/main';
+import { BackupArchiveInfo } from 'src/lib/import-export/import/types';
+import { createSite, startServer, isFullscreen, importSite } from '../ipc-handlers';
 import { isEmptyDir, pathExists } from '../lib/fs-utils';
+import { importBackup, defaultImporterOptions } from '../lib/import-export/import/import-manager';
 import { keepSqliteIntegrationUpdated } from '../lib/sqlite-versions';
 import { getMainWindow } from '../main-window';
 import { SiteServer, createSiteWorkingDirectory } from '../site-server';
@@ -17,6 +20,8 @@ jest.mock( '../site-server' );
 jest.mock( '../lib/sqlite-versions' );
 jest.mock( '../../vendor/wp-now/src/download' );
 jest.mock( '../main-window' );
+jest.mock( '@sentry/electron/main' );
+jest.mock( '../lib/import-export/import/import-manager' );
 
 ( SiteServer.create as jest.Mock ).mockImplementation( ( details ) => ( {
 	start: jest.fn(),
@@ -118,3 +123,75 @@ describe( 'isFullscreen', () => {
 		expect( result ).toBe( true );
 	} );
 } );
+
+describe( 'importSite', () => {
+	const mockBackupFile: BackupArchiveInfo = {
+		path: '/path/to/backup.zip',
+		type: 'doo',
+	};
+
+	beforeEach( () => {
+		( importBackup as jest.Mock ).mockReset();
+	} );
+
+	it( 'should throw error if site is not found', async () => {
+		( SiteServer.get as jest.Mock ).mockReturnValue( null );
+
+		await expect(
+			importSite( mockIpcMainInvokeEvent, {
+				id: 'non-existent-id',
+				backupFile: mockBackupFile,
+			} )
+		).rejects.toThrow( 'Site not found.' );
+	} );
+
+	it( 'should import backup successfully', async () => {
+		const mockSite = {
+			details: {
+				id: 'test-site',
+				phpVersion: '8.0',
+			},
+			updateSiteDetails: jest.fn(),
+		};
+		( SiteServer.get as jest.Mock ).mockReturnValue( mockSite );
+		( importBackup as jest.Mock ).mockResolvedValue( {
+			meta: {
+				phpVersion: '8.2',
+			},
+		} );
+
+		const result = await importSite( mockIpcMainInvokeEvent, {
+			id: 'test-site',
+			backupFile: mockBackupFile,
+		} );
+
+		expect( importBackup ).toHaveBeenCalledWith(
+			mockBackupFile,
+			mockSite.details,
+			expect.any( Function ),
+			defaultImporterOptions
+		);
+		expect( mockSite.details.phpVersion ).toBe( '8.2' );
+		expect( result ).toBe( mockSite.details );
+	} );
+
+	it( 'should capture exception in Sentry when import fails', async () => {
+		const mockError = new Error( 'Import failed' );
+		const mockSite = {
+			details: {
+				id: 'test-site',
+			},
+		};
+		( SiteServer.get as jest.Mock ).mockReturnValue( mockSite );
+		( importBackup as jest.Mock ).mockRejectedValue( mockError );
+
+		await expect(
+			importSite( mockIpcMainInvokeEvent, {
+				id: 'test-site',
+				backupFile: mockBackupFile,
+			} )
+		).rejects.toThrow( 'Import failed' );
+
+		expect( Sentry.captureException ).toHaveBeenCalledWith( mockError );
+	} );
+} );