mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
fix(brevilabs): rewrite uploads via requestUrl multipart (W8/9) (#2399)
* fix(brevilabs): rewrite uploads via requestUrl multipart (W8/9) Ninth of nine source workspaces splitting #2397. Replaces native fetch(FormData) calls in brevilabsClient with Obsidian's requestUrl plus a manually-constructed multipart body. This is required because Obsidian's CORS-bypass path can't handle native FormData streams. - src/LLMProviders/brevilabsClient.ts: buildMultipartFromFormData helper builds a Uint8Array body with explicit boundary; uploads now flow through requestUrl with a Content-Type header that includes the boundary string - __mocks__/obsidian.js: requestUrl jest mock + __setRequestUrlImpl helper so tests can swap in per-case responses Manual verification required: image upload, PDF upload, audio transcription, error path. Plus license needed to exercise real uploads. W0 already merged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(network): dedupe brevilabs response parsing + bound model-fetch timeout - Extract parseBrevilabsResponse helper so makeRequest and makeFormDataRequest share one parse/error path. - Wrap fetchModelsForProvider's safeFetch in a 10s Promise.race (safeFetch ignores AbortSignal, so a dead base URL could hang the model-import dialog forever). - Preserve real @/utils exports in selfHostServices test mock via jest.requireActual. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(lint): fix typescript-eslint violations in brevilabs + tests Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
19dd1aab40
commit
01a01e2951
6 changed files with 135 additions and 78 deletions
|
|
@ -1,10 +1,25 @@
|
||||||
// __mocks__/obsidian.js
|
// __mocks__/obsidian.js
|
||||||
import { parse as parseYamlString } from "yaml";
|
import { parse as parseYamlString } from "yaml";
|
||||||
|
|
||||||
|
// Per-test overrides set via the exported `__setRequestUrlImpl` helper.
|
||||||
|
// Default: empty success response. Tests that exercise network paths should
|
||||||
|
// install their own implementation.
|
||||||
|
let requestUrlImpl = jest.fn().mockResolvedValue({
|
||||||
|
status: 200,
|
||||||
|
text: "",
|
||||||
|
json: undefined,
|
||||||
|
arrayBuffer: new ArrayBuffer(0),
|
||||||
|
headers: {},
|
||||||
|
});
|
||||||
|
|
||||||
module.exports = {
|
module.exports = {
|
||||||
// Reason: normalizePath is used by projectPaths.ts; identity function is sufficient for tests
|
// Reason: normalizePath is used by projectPaths.ts; identity function is sufficient for tests
|
||||||
normalizePath: jest.fn().mockImplementation((p) => p),
|
normalizePath: jest.fn().mockImplementation((p) => p),
|
||||||
moment: jest.requireActual("moment"),
|
moment: jest.requireActual("moment"),
|
||||||
|
requestUrl: (...args) => requestUrlImpl(...args),
|
||||||
|
__setRequestUrlImpl: (impl) => {
|
||||||
|
requestUrlImpl = impl;
|
||||||
|
},
|
||||||
Vault: jest.fn().mockImplementation(() => {
|
Vault: jest.fn().mockImplementation(() => {
|
||||||
return {
|
return {
|
||||||
getMarkdownFiles: jest.fn().mockImplementation(() => {
|
getMarkdownFiles: jest.fn().mockImplementation(() => {
|
||||||
|
|
|
||||||
|
|
@ -1,3 +1,4 @@
|
||||||
|
import { safeFetchNoThrow } from "@/utils";
|
||||||
import { OpenAIEmbeddings } from "@langchain/openai";
|
import { OpenAIEmbeddings } from "@langchain/openai";
|
||||||
|
|
||||||
export class CustomOpenAIEmbeddings extends OpenAIEmbeddings {
|
export class CustomOpenAIEmbeddings extends OpenAIEmbeddings {
|
||||||
|
|
@ -35,7 +36,7 @@ export class CustomOpenAIEmbeddings extends OpenAIEmbeddings {
|
||||||
const baseURL = configuration?.baseURL || "https://api.openai.com/v1";
|
const baseURL = configuration?.baseURL || "https://api.openai.com/v1";
|
||||||
const url = `${baseURL}/embeddings`;
|
const url = `${baseURL}/embeddings`;
|
||||||
const apiKey = this.customConfig.apiKey as string;
|
const apiKey = this.customConfig.apiKey as string;
|
||||||
const fetchFn = configuration?.fetch || fetch;
|
const fetchFn = configuration?.fetch || safeFetchNoThrow;
|
||||||
|
|
||||||
const response = await fetchFn(url, {
|
const response = await fetchFn(url, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
|
|
|
||||||
|
|
@ -4,8 +4,86 @@ import { MissingPlusLicenseError } from "@/error";
|
||||||
import { logInfo } from "@/logger";
|
import { logInfo } from "@/logger";
|
||||||
import { turnOffPlus, turnOnPlus } from "@/plusUtils";
|
import { turnOffPlus, turnOnPlus } from "@/plusUtils";
|
||||||
import { getSettings } from "@/settings/model";
|
import { getSettings } from "@/settings/model";
|
||||||
import { safeFetchNoThrow } from "@/utils";
|
|
||||||
import { arrayBufferToBase64 } from "@/utils/base64";
|
import { arrayBufferToBase64 } from "@/utils/base64";
|
||||||
|
import { requestUrl } from "obsidian";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Build a multipart/form-data body buffer from a FormData instance.
|
||||||
|
* Returned as an ArrayBuffer suitable for passing to Obsidian's requestUrl.
|
||||||
|
*
|
||||||
|
* @param formData - FormData containing strings and/or File/Blob entries.
|
||||||
|
* @returns The serialized multipart body and the Content-Type header (including boundary).
|
||||||
|
*/
|
||||||
|
async function buildMultipartFromFormData(
|
||||||
|
formData: FormData
|
||||||
|
): Promise<{ body: ArrayBuffer; contentType: string }> {
|
||||||
|
const boundary = `----CopilotBoundary${Math.random().toString(16).slice(2)}${Date.now().toString(16)}`;
|
||||||
|
const encoder = new TextEncoder();
|
||||||
|
const parts: Uint8Array[] = [];
|
||||||
|
|
||||||
|
for (const [name, value] of formData.entries()) {
|
||||||
|
parts.push(encoder.encode(`--${boundary}\r\n`));
|
||||||
|
if (value instanceof Blob) {
|
||||||
|
const filename = value instanceof File ? value.name : "blob";
|
||||||
|
const contentType = value.type || "application/octet-stream";
|
||||||
|
parts.push(
|
||||||
|
encoder.encode(
|
||||||
|
`Content-Disposition: form-data; name="${name}"; filename="${filename}"\r\n` +
|
||||||
|
`Content-Type: ${contentType}\r\n\r\n`
|
||||||
|
)
|
||||||
|
);
|
||||||
|
const buf = await value.arrayBuffer();
|
||||||
|
parts.push(new Uint8Array(buf));
|
||||||
|
parts.push(encoder.encode("\r\n"));
|
||||||
|
} else {
|
||||||
|
parts.push(encoder.encode(`Content-Disposition: form-data; name="${name}"\r\n\r\n`));
|
||||||
|
parts.push(encoder.encode(String(value)));
|
||||||
|
parts.push(encoder.encode("\r\n"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
parts.push(encoder.encode(`--${boundary}--\r\n`));
|
||||||
|
|
||||||
|
const totalLength = parts.reduce((sum, p) => sum + p.byteLength, 0);
|
||||||
|
const out = new Uint8Array(totalLength);
|
||||||
|
let offset = 0;
|
||||||
|
for (const part of parts) {
|
||||||
|
out.set(part, offset);
|
||||||
|
offset += part.byteLength;
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
body: out.buffer.slice(out.byteOffset, out.byteOffset + out.byteLength),
|
||||||
|
contentType: `multipart/form-data; boundary=${boundary}`,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Normalize a requestUrl response into the {data, error} shape used by Brevilabs API methods.
|
||||||
|
* Handles the case where `response.json` is a raw string (non-JSON body, e.g. HTML error page).
|
||||||
|
*/
|
||||||
|
function parseBrevilabsResponse<T>(
|
||||||
|
response: { status: number; json: unknown },
|
||||||
|
endpoint: string
|
||||||
|
): { data: T | null; error?: Error } {
|
||||||
|
let data: unknown = response.json;
|
||||||
|
if (typeof data === "string") {
|
||||||
|
try {
|
||||||
|
data = JSON.parse(data);
|
||||||
|
} catch {
|
||||||
|
// Non-JSON body — fall through to status-based error.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (response.status < 200 || response.status >= 300) {
|
||||||
|
const detail = (data as { detail?: { reason?: string; error?: string } } | null)?.detail;
|
||||||
|
if (detail?.reason) {
|
||||||
|
const error = new Error(detail.reason);
|
||||||
|
if (detail.error) error.name = detail.error;
|
||||||
|
return { data: null, error };
|
||||||
|
}
|
||||||
|
return { data: null, error: new Error(`HTTP error: ${response.status}`) };
|
||||||
|
}
|
||||||
|
logInfo(`[API ${endpoint} request]:`, data);
|
||||||
|
return { data: data as T };
|
||||||
|
}
|
||||||
|
|
||||||
export interface RerankResponse {
|
export interface RerankResponse {
|
||||||
response: {
|
response: {
|
||||||
|
|
@ -123,25 +201,14 @@ export class BrevilabsClient {
|
||||||
if (!excludeAuthHeader) {
|
if (!excludeAuthHeader) {
|
||||||
headers.Authorization = `Bearer ${await getDecryptedKey(getSettings().plusLicenseKey)}`;
|
headers.Authorization = `Bearer ${await getDecryptedKey(getSettings().plusLicenseKey)}`;
|
||||||
}
|
}
|
||||||
const response = await safeFetchNoThrow(url.toString(), {
|
const response = await requestUrl({
|
||||||
|
url: url.toString(),
|
||||||
method,
|
method,
|
||||||
headers,
|
headers,
|
||||||
...(method === "POST" && { body: JSON.stringify(body) }),
|
...(method === "POST" && { body: JSON.stringify(body) }),
|
||||||
|
throw: false,
|
||||||
});
|
});
|
||||||
const data = await response.json();
|
return parseBrevilabsResponse<T>(response, endpoint);
|
||||||
if (!response.ok) {
|
|
||||||
try {
|
|
||||||
const errorDetail = data.detail;
|
|
||||||
const error = new Error(errorDetail.reason as string);
|
|
||||||
error.name = errorDetail.error as string;
|
|
||||||
return { data: null, error };
|
|
||||||
} catch {
|
|
||||||
return { data: null, error: new Error("Unknown error") };
|
|
||||||
}
|
|
||||||
}
|
|
||||||
logInfo(`[API ${endpoint} request]:`, data);
|
|
||||||
|
|
||||||
return { data };
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private async makeFormDataRequest<T>(
|
private async makeFormDataRequest<T>(
|
||||||
|
|
@ -159,29 +226,21 @@ export class BrevilabsClient {
|
||||||
const url = new URL(`${BREVILABS_API_BASE_URL}${endpoint}`);
|
const url = new URL(`${BREVILABS_API_BASE_URL}${endpoint}`);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const response = await fetch(url.toString(), {
|
// Build multipart body manually for requestUrl (does not natively support FormData).
|
||||||
|
const { body, contentType } = await buildMultipartFromFormData(formData);
|
||||||
|
|
||||||
|
const response = await requestUrl({
|
||||||
|
url: url.toString(),
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: {
|
headers: {
|
||||||
// No Content-Type header - browser will set it automatically with boundary
|
"Content-Type": contentType,
|
||||||
Authorization: `Bearer ${await getDecryptedKey(getSettings().plusLicenseKey)}`,
|
Authorization: `Bearer ${await getDecryptedKey(getSettings().plusLicenseKey)}`,
|
||||||
"X-Client-Version": this.pluginVersion,
|
"X-Client-Version": this.pluginVersion,
|
||||||
},
|
},
|
||||||
body: formData,
|
body,
|
||||||
|
throw: false,
|
||||||
});
|
});
|
||||||
|
return parseBrevilabsResponse<T>(response, `${endpoint} form-data`);
|
||||||
const data = await response.json();
|
|
||||||
if (!response.ok) {
|
|
||||||
try {
|
|
||||||
const errorDetail = data.detail;
|
|
||||||
const error = new Error(errorDetail.reason as string);
|
|
||||||
error.name = errorDetail.error as string;
|
|
||||||
return { data: null, error };
|
|
||||||
} catch {
|
|
||||||
return { data: null, error: new Error(`HTTP error: ${response.status}`) };
|
|
||||||
}
|
|
||||||
}
|
|
||||||
logInfo(`[API ${endpoint} form-data request]:`, data);
|
|
||||||
return { data };
|
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
return { data: null, error: error instanceof Error ? error : new Error(String(error)) };
|
return { data: null, error: error instanceof Error ? error : new Error(String(error)) };
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -17,9 +17,15 @@ jest.mock("@/logger", () => ({
|
||||||
logWarn: jest.fn(),
|
logWarn: jest.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
// Mock global fetch
|
// Mock safeFetchNoThrow (the requestUrl-backed wrapper used in place of fetch)
|
||||||
const mockFetch = jest.fn();
|
const mockFetch = jest.fn();
|
||||||
window.fetch = mockFetch;
|
jest.mock("@/utils", () => {
|
||||||
|
const actual = jest.requireActual<Record<string, unknown>>("@/utils");
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
safeFetchNoThrow: (url: string, options?: RequestInit): unknown => mockFetch(url, options),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
jest.clearAllMocks();
|
jest.clearAllMocks();
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ import { type Youtube4llmResponse } from "@/LLMProviders/brevilabsClient";
|
||||||
import { getDecryptedKey } from "@/encryptionService";
|
import { getDecryptedKey } from "@/encryptionService";
|
||||||
import { logError, logInfo } from "@/logger";
|
import { logError, logInfo } from "@/logger";
|
||||||
import { getSettings } from "@/settings/model";
|
import { getSettings } from "@/settings/model";
|
||||||
|
import { safeFetchNoThrow } from "@/utils";
|
||||||
|
|
||||||
const FIRECRAWL_SEARCH_URL = "https://api.firecrawl.dev/v2/search";
|
const FIRECRAWL_SEARCH_URL = "https://api.firecrawl.dev/v2/search";
|
||||||
const PERPLEXITY_CHAT_URL = "https://api.perplexity.ai/chat/completions";
|
const PERPLEXITY_CHAT_URL = "https://api.perplexity.ai/chat/completions";
|
||||||
|
|
@ -45,7 +46,7 @@ export function hasSelfHostSearchKey(): boolean {
|
||||||
async function firecrawlSearch(query: string, apiKey: string): Promise<SelfHostWebSearchResult> {
|
async function firecrawlSearch(query: string, apiKey: string): Promise<SelfHostWebSearchResult> {
|
||||||
const startTime = Date.now();
|
const startTime = Date.now();
|
||||||
|
|
||||||
const response = await fetch(FIRECRAWL_SEARCH_URL, {
|
const response = await safeFetchNoThrow(FIRECRAWL_SEARCH_URL, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: {
|
headers: {
|
||||||
Authorization: `Bearer ${apiKey}`,
|
Authorization: `Bearer ${apiKey}`,
|
||||||
|
|
@ -95,7 +96,7 @@ async function perplexitySonarSearch(
|
||||||
query: string,
|
query: string,
|
||||||
apiKey: string
|
apiKey: string
|
||||||
): Promise<SelfHostWebSearchResult> {
|
): Promise<SelfHostWebSearchResult> {
|
||||||
const response = await fetch(PERPLEXITY_CHAT_URL, {
|
const response = await safeFetchNoThrow(PERPLEXITY_CHAT_URL, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: {
|
headers: {
|
||||||
Authorization: `Bearer ${apiKey}`,
|
Authorization: `Bearer ${apiKey}`,
|
||||||
|
|
@ -144,7 +145,7 @@ export async function selfHostYoutube4llm(url: string): Promise<Youtube4llmRespo
|
||||||
|
|
||||||
const transcriptUrl = `${SUPADATA_TRANSCRIPT_URL}?url=${encodeURIComponent(url)}&mode=auto&text=true`;
|
const transcriptUrl = `${SUPADATA_TRANSCRIPT_URL}?url=${encodeURIComponent(url)}&mode=auto&text=true`;
|
||||||
|
|
||||||
const response = await fetch(transcriptUrl, {
|
const response = await safeFetchNoThrow(transcriptUrl, {
|
||||||
method: "GET",
|
method: "GET",
|
||||||
headers: {
|
headers: {
|
||||||
"x-api-key": apiKey,
|
"x-api-key": apiKey,
|
||||||
|
|
@ -189,7 +190,7 @@ async function pollSupadataJob(
|
||||||
while (Date.now() < deadline) {
|
while (Date.now() < deadline) {
|
||||||
await new Promise((resolve) => window.setTimeout(resolve, SUPADATA_POLL_INTERVAL));
|
await new Promise((resolve) => window.setTimeout(resolve, SUPADATA_POLL_INTERVAL));
|
||||||
|
|
||||||
const pollResponse = await fetch(pollUrl, {
|
const pollResponse = await safeFetchNoThrow(pollUrl, {
|
||||||
method: "GET",
|
method: "GET",
|
||||||
headers: {
|
headers: {
|
||||||
"x-api-key": apiKey,
|
"x-api-key": apiKey,
|
||||||
|
|
|
||||||
|
|
@ -3,7 +3,7 @@ import { ChatModelProviders, SettingKeyProviders } from "@/constants";
|
||||||
import { getDecryptedKey } from "@/encryptionService";
|
import { getDecryptedKey } from "@/encryptionService";
|
||||||
import { GitHubCopilotProvider } from "@/LLMProviders/githubCopilot/GitHubCopilotProvider";
|
import { GitHubCopilotProvider } from "@/LLMProviders/githubCopilot/GitHubCopilotProvider";
|
||||||
import ProjectManager from "@/LLMProviders/projectManager";
|
import ProjectManager from "@/LLMProviders/projectManager";
|
||||||
import { logError, logWarn } from "@/logger";
|
import { logError } from "@/logger";
|
||||||
import { parseModelsResponse, StandardModel } from "@/settings/providerModels";
|
import { parseModelsResponse, StandardModel } from "@/settings/providerModels";
|
||||||
import { err2String, getProviderInfo, safeFetch } from "@/utils";
|
import { err2String, getProviderInfo, safeFetch } from "@/utils";
|
||||||
import { getApiKeyForProvider } from "@/utils/modelUtils";
|
import { getApiKeyForProvider } from "@/utils/modelUtils";
|
||||||
|
|
@ -66,43 +66,18 @@ export async function fetchModelsForProvider(
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
const tryFetch = async (useSafeFetch: boolean) => {
|
// Use safeFetch (requestUrl) to bypass CORS on desktop and mobile. safeFetch
|
||||||
const controller = new AbortController();
|
// does not honor AbortSignal, so bound the call manually via Promise.race.
|
||||||
const timeoutId = window.setTimeout(() => controller.abort(), 3000);
|
const response = await Promise.race([
|
||||||
|
safeFetch(url, { headers, method: "GET" }),
|
||||||
try {
|
new Promise<never>((_, reject) =>
|
||||||
const response = await (useSafeFetch ? safeFetch : fetch)(url, {
|
window.setTimeout(() => reject(new Error("Request timed out after 10s")), 10000)
|
||||||
headers,
|
),
|
||||||
signal: controller.signal,
|
]);
|
||||||
method: "GET",
|
if (!response.ok) {
|
||||||
});
|
const msg = err2String(await response.json());
|
||||||
|
logError(msg);
|
||||||
if (!response.ok) {
|
throw new Error(`Failed to fetch models: ${response.statusText} \n detail: ` + msg);
|
||||||
const msg = err2String(await response.json());
|
|
||||||
logError(msg);
|
|
||||||
throw new Error(`Failed to fetch models: ${response.statusText} \n detail: ` + msg);
|
|
||||||
}
|
|
||||||
return response;
|
|
||||||
} finally {
|
|
||||||
window.clearTimeout(timeoutId);
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
let response;
|
|
||||||
try {
|
|
||||||
response = await tryFetch(false);
|
|
||||||
} catch (firstError) {
|
|
||||||
logWarn("First fetch attempt failed, trying with safeFetch...");
|
|
||||||
try {
|
|
||||||
response = await tryFetch(true);
|
|
||||||
} catch (error) {
|
|
||||||
const msg =
|
|
||||||
"\nwithout CORS Error: " +
|
|
||||||
err2String(firstError) +
|
|
||||||
"\nwith CORS Error: " +
|
|
||||||
err2String(error);
|
|
||||||
throw new Error(msg);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const rawData = await response.json();
|
const rawData = await response.json();
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue