Skip to content

fix: 优化文件下载器方法 #6617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+92 −2
Open

fix: 优化文件下载器方法 #6617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df65501
fix: 优化文件下载器方法
lghuahua Aug 8, 2025
fddfc6d
fix: 增加对不支持的 HTTP 方法的错误处理
lghuahua Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions packages/effects/request/src/request-client/modules/downloader.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe('fileDownloader', () => {
expect(result).toBeInstanceOf(Blob);
expect(result).toEqual(mockBlob);
expect(mockAxiosInstance.get).toHaveBeenCalledWith(url, {
method: 'GET',
responseType: 'blob',
responseReturn: 'body',
});
Expand All @@ -51,6 +52,7 @@ describe('fileDownloader', () => {
expect(result).toEqual(mockBlob);
expect(mockAxiosInstance.get).toHaveBeenCalledWith(url, {
...customConfig,
method: 'GET',
responseType: 'blob',
responseReturn: 'body',
});
Expand Down Expand Up @@ -84,3 +86,72 @@ describe('fileDownloader', () => {
);
});
});

describe('fileDownloader use other method', () => {
let fileDownloader: FileDownloader;

it('should call request using get', async () => {
const url = 'https://example.com/file';
const mockBlob = new Blob(['file content'], { type: 'text/plain' });
const mockResponse: Blob = mockBlob;

const mockAxiosInstance = {
request: vi.fn(),
} as any;

fileDownloader = new FileDownloader(mockAxiosInstance);

mockAxiosInstance.request.mockResolvedValueOnce(mockResponse);

const result = await fileDownloader.download(url);

expect(result).toBeInstanceOf(Blob);
expect(result).toEqual(mockBlob);
expect(mockAxiosInstance.request).toHaveBeenCalledWith(url, {
method: 'GET',
responseType: 'blob',
responseReturn: 'body',
});
});

it('should call post', async () => {
const url = 'https://example.com/file';

const mockAxiosInstance = {
post: vi.fn(),
} as any;

fileDownloader = new FileDownloader(mockAxiosInstance);

const customConfig: AxiosRequestConfig = {
method: 'POST',
data: { name: 'aa' },
};

await fileDownloader.download(url, customConfig);

expect(mockAxiosInstance.post).toHaveBeenCalledWith(
url,
{ name: 'aa' },
{
method: 'POST',
responseType: 'blob',
responseReturn: 'body',
},
);
});

it('should handle errors gracefully', async () => {
const url = 'https://example.com/file';
const mockAxiosInstance = {
post: vi.fn(),
} as any;

fileDownloader = new FileDownloader(mockAxiosInstance);
await expect(() =>
fileDownloader.download(url, { method: 'postt' }),
).rejects.toThrow(
'RequestClient does not support method "POSTT". Please ensure the method is properly implemented in your RequestClient instance.',
);
Comment on lines +151 to +155
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix async rejection assertion (use a Promise, not a function).

rejects matcher expects a Promise: wrap the awaited call directly. Current form may not fail as intended.

Apply this diff:

-    await expect(() =>
-      fileDownloader.download(url, { method: 'postt' }),
-    ).rejects.toThrow(
+    await expect(
+      fileDownloader.download(url, { method: 'postt' }),
+    ).rejects.toThrow(
       'RequestClient does not support method "POSTT". Please ensure the method is properly implemented in your RequestClient instance.',
     );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await expect(() =>
fileDownloader.download(url, { method: 'postt' }),
).rejects.toThrow(
'RequestClient does not support method "POSTT". Please ensure the method is properly implemented in your RequestClient instance.',
);
await expect(
fileDownloader.download(url, { method: 'postt' }),
).rejects.toThrow(
'RequestClient does not support method "POSTT". Please ensure the method is properly implemented in your RequestClient instance.',
);
🤖 Prompt for AI Agents 
In packages/effects/request/src/request-client/modules/downloader.test.ts around
lines 151 to 155, the test uses expect(() => await ...) with the rejects matcher
which expects a Promise; change the assertion to pass the Promise directly by
calling fileDownloader.download(...) (no wrapper function) and chain
.rejects.toThrow(...) (ensure you do not await the promise before passing it to
expect). This fixes the async rejection assertion so the test correctly verifies
the download promise rejects with the expected error message.

});
});
23 changes: 21 additions & 2 deletions packages/effects/request/src/request-client/modules/downloader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,32 @@ class FileDownloader {
): Promise<T> {
const finalConfig: DownloadRequestConfig = {
responseReturn: 'body',
method: 'GET',
...config,
responseType: 'blob',
};

const response = await this.client.get<T>(url, finalConfig);
// Prefer a generic request if available; otherwise, dispatch to method-specific calls.
const method = (finalConfig.method || 'GET').toUpperCase();
const clientAny = this.client as any;

return response;
if (typeof clientAny.request === 'function') {
return await clientAny.request(url, finalConfig);
}
const lower = method.toLowerCase();

if (typeof clientAny[lower] === 'function') {
if (['POST', 'PUT'].includes(method)) {
const { data, ...rest } = finalConfig as Record<string, any>;
return await clientAny[lower](url, data, rest);
}

return await clientAny[lower](url, finalConfig);
}
Comment on lines +45 to +52
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

PATCH is treated like GET here — body is dropped.

Axios-style patch(url, data, config) requires passing data separately. Current code only special‑cases POST and PUT, so PATCH will incorrectly call client.patch(url, finalConfig), losing the request body.

Apply this diff to include PATCH and avoid sending data with GET:

-    if (typeof clientAny[lower] === 'function') {
-      if (['POST', 'PUT'].includes(method)) {
-        const { data, ...rest } = finalConfig as Record<string, any>;
-        return await clientAny[lower](url, data, rest);
-      }
-
-      return await clientAny[lower](url, finalConfig);
-    }
+    if (typeof clientAny[lower] === 'function') {
+      if (['POST', 'PUT', 'PATCH'].includes(method)) {
+        const { data, ...rest } = finalConfig as Record<string, any>;
+        return await clientAny[lower](url, data, rest);
+      }
+      if (method === 'GET') {
+        // Avoid accidental bodies on GET
+        const { data: _ignored, ...rest } = finalConfig as Record<string, any>;
+        return await clientAny[lower](url, rest);
+      }
+      return await clientAny[lower](url, finalConfig);
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (typeof clientAny[lower] === 'function') {
if (['POST', 'PUT'].includes(method)) {
const { data, ...rest } = finalConfig as Record<string, any>;
return await clientAny[lower](url, data, rest);
}
return await clientAny[lower](url, finalConfig);
}
if (typeof clientAny[lower] === 'function') {
if (['POST', 'PUT', 'PATCH'].includes(method)) {
const { data, ...rest } = finalConfig as Record<string, any>;
return await clientAny[lower](url, data, rest);
}
if (method === 'GET') {
// Avoid accidental bodies on GET
const { data: _ignored, ...rest } = finalConfig as Record<string, any>;
return await clientAny[lower](url, rest);
}
return await clientAny[lower](url, finalConfig);
}
🤖 Prompt for AI Agents 
In packages/effects/request/src/request-client/modules/downloader.ts around
lines 45 to 52, the PATCH method is currently treated like GET and its request
body is dropped; update the conditional that special-cases methods with a
request body to include 'PATCH' (i.e., treat POST, PUT, PATCH the same) and
destructure finalConfig into { data, ...rest } for those methods so you call
client[lower](url, data, rest); for methods that do not send a body (GET,
DELETE, etc.) call client[lower](url, finalConfig) so data is not sent
incorrectly.


throw new Error(
`RequestClient does not support method "${method}". Please ensure the method is properly implemented in your RequestClient instance.`,
);
}
}

Expand Down
Loading