fix: #3727 LLM model download fail can still be used (#3731)

* fix: #3727 - LLM model download fail can still be used

* test: add tests

* test: fix path on Windows

core/src/node/api/processors/download.test.ts

@@ -1,59 +1,131 @@
-import { Downloader } from './download';
-import { DownloadEvent } from '../../../types/api';
-import { DownloadManager } from '../../helper/download';
+import { Downloader } from './download'
+import { DownloadEvent } from '../../../types/api'
+import { DownloadManager } from '../../helper/download'
 
-it('should handle getFileSize errors correctly', async () => {
-  const observer = jest.fn();
-  const url = 'http://example.com/file';
+jest.mock('../../helper', () => ({
+  getJanDataFolderPath: jest.fn().mockReturnValue('path/to/folder'),
+}))
 
-  const downloader = new Downloader(observer);
-  const requestMock = jest.fn((options, callback) => {
-    callback(new Error('Test error'), null);
-  });
-  jest.mock('request', () => requestMock);
+jest.mock('../../helper/path', () => ({
+  validatePath: jest.fn().mockReturnValue('path/to/folder'),
+  normalizeFilePath: () => process.platform === 'win32' ? 'C:\\Users\path\\to\\file.gguf' : '/Users/path/to/file.gguf',
+}))
 
-  await expect(downloader.getFileSize(observer, url)).rejects.toThrow('Test error');
-});
+jest.mock(
+  'request',
+  jest.fn().mockReturnValue(() => ({
+    on: jest.fn(),
+  }))
+)
 
+jest.mock('fs', () => ({
+  createWriteStream: jest.fn(),
+}))
 
-it('should pause download correctly', () => {
-  const observer = jest.fn();
-  const fileName = process.platform === 'win32' ? 'C:\\path\\to\\file' : 'path/to/file';
+jest.mock('request-progress', () => {
+  return jest.fn().mockImplementation(() => {
+    return {
+      on: jest.fn().mockImplementation((event, callback) => {
+        if (event === 'error') {
+          callback(new Error('Download failed'))
+        }
+        return {
+          on: jest.fn().mockImplementation((event, callback) => {
+            if (event === 'error') {
+              callback(new Error('Download failed'))
+            }
+            return {
+              on: jest.fn().mockImplementation((event, callback) => {
+                if (event === 'error') {
+                  callback(new Error('Download failed'))
+                }
+                return { pipe: jest.fn() }
+              }),
+            }
+          }),
+        }
+      }),
+    }
+  })
+})
 
-  const downloader = new Downloader(observer);
-  const pauseMock = jest.fn();
-  DownloadManager.instance.networkRequests[fileName] = { pause: pauseMock };
+describe('Downloader', () => {
+  beforeEach(() => {
+    jest.resetAllMocks()
+  })
+  it('should handle getFileSize errors correctly', async () => {
+    const observer = jest.fn()
+    const url = 'http://example.com/file'
 
-  downloader.pauseDownload(observer, fileName);
+    const downloader = new Downloader(observer)
+    const requestMock = jest.fn((options, callback) => {
+      callback(new Error('Test error'), null)
+    })
+    jest.mock('request', () => requestMock)
 
-  expect(pauseMock).toHaveBeenCalled();
-});
+    await expect(downloader.getFileSize(observer, url)).rejects.toThrow('Test error')
+  })
 
-it('should resume download correctly', () => {
-  const observer = jest.fn();
-  const fileName = process.platform === 'win32' ? 'C:\\path\\to\\file' : 'path/to/file';
+  it('should pause download correctly', () => {
+    const observer = jest.fn()
+    const fileName = process.platform === 'win32' ? 'C:\\path\\to\\file' : 'path/to/file'
 
-  const downloader = new Downloader(observer);
-  const resumeMock = jest.fn();
-  DownloadManager.instance.networkRequests[fileName] = { resume: resumeMock };
+    const downloader = new Downloader(observer)
+    const pauseMock = jest.fn()
+    DownloadManager.instance.networkRequests[fileName] = { pause: pauseMock }
 
-  downloader.resumeDownload(observer, fileName);
+    downloader.pauseDownload(observer, fileName)
 
-  expect(resumeMock).toHaveBeenCalled();
-});
+    expect(pauseMock).toHaveBeenCalled()
+  })
 
-it('should handle aborting a download correctly', () => {
-  const observer = jest.fn();
-  const fileName = process.platform === 'win32' ? 'C:\\path\\to\\file' : 'path/to/file';
+  it('should resume download correctly', () => {
+    const observer = jest.fn()
+    const fileName = process.platform === 'win32' ? 'C:\\path\\to\\file' : 'path/to/file'
 
-  const downloader = new Downloader(observer);
-  const abortMock = jest.fn();
-  DownloadManager.instance.networkRequests[fileName] = { abort: abortMock };
+    const downloader = new Downloader(observer)
+    const resumeMock = jest.fn()
+    DownloadManager.instance.networkRequests[fileName] = { resume: resumeMock }
 
-  downloader.abortDownload(observer, fileName);
+    downloader.resumeDownload(observer, fileName)
 
-  expect(abortMock).toHaveBeenCalled();
-  expect(observer).toHaveBeenCalledWith(DownloadEvent.onFileDownloadError, expect.objectContaining({
-    error: 'aborted'
-  }));
-});
+    expect(resumeMock).toHaveBeenCalled()
+  })
+
+  it('should handle aborting a download correctly', () => {
+    const observer = jest.fn()
+    const fileName = process.platform === 'win32' ? 'C:\\path\\to\\file' : 'path/to/file'
+
+    const downloader = new Downloader(observer)
+    const abortMock = jest.fn()
+    DownloadManager.instance.networkRequests[fileName] = { abort: abortMock }
+
+    downloader.abortDownload(observer, fileName)
+
+    expect(abortMock).toHaveBeenCalled()
+    expect(observer).toHaveBeenCalledWith(
+      DownloadEvent.onFileDownloadError,
+      expect.objectContaining({
+        error: 'aborted',
+      })
+    )
+  })
+
+  it('should handle download fail correctly', () => {
+    const observer = jest.fn()
+    const fileName = process.platform === 'win32' ? 'C:\\path\\to\\file' : 'path/to/file.gguf'
+
+    const downloader = new Downloader(observer)
+
+    downloader.downloadFile(observer, {
+      localPath: fileName,
+      url: 'http://127.0.0.1',
+    })
+    expect(observer).toHaveBeenCalledWith(
+      DownloadEvent.onFileDownloadError,
+      expect.objectContaining({
+        error: expect.anything(),
+      })
+    )
+  })
+})

core/src/node/api/processors/download.ts

@@ -100,7 +100,11 @@ export class Downloader implements Processor {
       })
       .on('end', () => {
         const currentDownloadState = DownloadManager.instance.downloadProgressMap[modelId]
-        if (currentDownloadState && DownloadManager.instance.networkRequests[normalizedPath]) {
+        if (
+          currentDownloadState &&
+          DownloadManager.instance.networkRequests[normalizedPath] &&
+          DownloadManager.instance.downloadProgressMap[modelId]?.downloadState !== 'error'
+        ) {
           // Finished downloading, rename temp file to actual file
           renameSync(downloadingTempFile, destination)
           const downloadState: DownloadState = {