Fix file resolver on redirect (#1870)

* fix redirects in file resolver, improve errors

* add file resolver tests

* fixes #1869

packages/cli/src/command-helpers/file-resolver.test.ts

@@ -0,0 +1,125 @@
+import os from 'node:os';
+import path from 'node:path';
+import fs from 'fs-extra';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { resolveFile } from './file-resolver';
+
+// Mock fetch globally
+const mockFetch = vi.fn();
+global.fetch = mockFetch;
+
+describe('resolveFile', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  afterEach(async () => {
+    // Clean up any temp directories that might have been created
+    const dirs = await fs.readdir(os.tmpdir());
+    for (const dir of dirs) {
+      if (dir.startsWith('graph-file-')) {
+        await fs.remove(path.join(os.tmpdir(), dir));
+      }
+    }
+  });
+
+  it('should handle local files', async () => {
+    const testFile = path.join(os.tmpdir(), 'test.txt');
+    await fs.writeFile(testFile, 'test content');
+
+    const result = await resolveFile(testFile, 'test.txt');
+    expect(result.path).toBe(testFile);
+    expect(result.cleanup).toBeUndefined();
+
+    await fs.remove(testFile);
+  });
+
+  it('should handle HTTP URLs with redirects', async () => {
+    const testUrl = 'https://example.com/file.txt';
+    const testContent = 'test content';
+
+    mockFetch.mockResolvedValueOnce({
+      ok: true,
+      arrayBuffer: async () => Buffer.from(testContent),
+    });
+
+    const result = await resolveFile(testUrl, 'file.txt');
+
+    expect(mockFetch).toHaveBeenCalledWith(testUrl, { redirect: 'follow' });
+    expect(result.path).toMatch(/graph-file-.*\/file\.txt$/);
+    expect(result.cleanup).toBeDefined();
+    expect(await fs.readFile(result.path, 'utf-8')).toBe(testContent);
+
+    if (result.cleanup) {
+      result.cleanup();
+    }
+  });
+
+  it('should handle IPFS hashes', async () => {
+    const ipfsHash = 'QmTest';
+    const testContent = 'test content';
+
+    mockFetch.mockResolvedValueOnce({
+      ok: true,
+      arrayBuffer: async () => Buffer.from(testContent),
+    });
+
+    const result = await resolveFile(ipfsHash, 'file.txt');
+
+    expect(mockFetch).toHaveBeenCalledWith(expect.stringContaining(ipfsHash));
+    expect(result.path).toMatch(/graph-file-.*\/file\.txt$/);
+    expect(result.cleanup).toBeDefined();
+    expect(await fs.readFile(result.path, 'utf-8')).toBe(testContent);
+
+    if (result.cleanup) {
+      result.cleanup();
+    }
+  });
+
+  it('should handle failed HTTP fetches', async () => {
+    const testUrl = 'https://example.com/file.txt';
+
+    mockFetch.mockResolvedValueOnce({
+      ok: false,
+      statusText: 'Not Found',
+    });
+
+    await expect(resolveFile(testUrl, 'file.txt')).rejects.toThrow(
+      `Failed to resolve ${testUrl} - failed to fetch from URL: Not Found`,
+    );
+  });
+
+  it('should handle network errors', async () => {
+    const testUrl = 'https://example.com/file.txt';
+
+    mockFetch.mockRejectedValueOnce(new Error('Network error'));
+
+    await expect(resolveFile(testUrl, 'file.txt')).rejects.toThrow(
+      `Failed to resolve ${testUrl} - Network error`,
+    );
+  });
+
+  it('should handle timeout', async () => {
+    const testUrl = 'https://example.com/file.txt';
+
+    mockFetch.mockImplementationOnce(() => new Promise(resolve => setTimeout(resolve, 2000)));
+
+    await expect(resolveFile(testUrl, 'file.txt', 100)).rejects.toThrow('File download timed out');
+  });
+
+  it('should clean up temp files on error', async () => {
+    const testUrl = 'https://example.com/file.txt';
+
+    mockFetch.mockRejectedValueOnce(new Error('Network error'));
+
+    try {
+      await resolveFile(testUrl, 'file.txt');
+    } catch (e) {
+      // Expected error
+    }
+
+    const dirs = await fs.readdir(os.tmpdir());
+    const graphTempDirs = dirs.filter(dir => dir.startsWith('graph-file-'));
+    expect(graphTempDirs).toHaveLength(0);
+  });
+});

packages/cli/src/command-helpers/file-resolver.ts

@@ -1,7 +1,6 @@
 import os from 'node:os';
 import path from 'node:path';
 import fs from 'fs-extra';
-import fetch from '../fetch.js';
 import { DEFAULT_IPFS_URL } from './ipfs.js';
 
 export interface FileSource {
@@ -42,7 +41,7 @@ export async function resolveFile(
       if (source.startsWith('Qm')) {
         const response = await fetch(`${DEFAULT_IPFS_URL}/cat?arg=${source}`);
         if (!response.ok) {
-          throw new Error(`Failed to fetch file from IPFS: ${response.statusText}`);
+          throw new Error(`failed to fetch from IPFS: ${response.statusText}`);
         }
         const filePath = path.join(tempDir, fileName);
         const buffer = Buffer.from(await response.arrayBuffer());
@@ -52,9 +51,9 @@ export async function resolveFile(
 
       // If it's a URL
       if (source.startsWith('http')) {
-        const response = await fetch(source);
+        const response = await fetch(source, { redirect: 'follow' });
         if (!response.ok) {
-          throw new Error(`Failed to fetch file from URL: ${response.statusText}`);
+          throw new Error(`failed to fetch from URL: ${response.statusText}`);
         }
         const filePath = path.join(tempDir, fileName);
         const buffer = Buffer.from(await response.arrayBuffer());
@@ -66,7 +65,10 @@ export async function resolveFile(
       throw new Error('Invalid file source. Must be a file path, IPFS hash, or URL');
     } catch (error) {
       cleanup();
-      throw error;
+      if (error instanceof Error) {
+        throw new Error(`Failed to resolve ${source} - ${error.message}`);
+      }
+      throw new Error(`Failed to resolve ${source}`);
     }
   };