Skip to content
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

feat: [#5714] 支持GLM #5741

Merged
merged 4 commits into from
Nov 1, 2024
Merged

feat: [#5714] 支持GLM #5741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d357b45
feat: [#5714] 支持GLM
DDMeaqua Oct 30, 2024
d3f0a77
chore: update Provider
DDMeaqua Oct 31, 2024
7a8d557
chore: 开启插件
DDMeaqua Oct 31, 2024
afe12c2
chore: update
DDMeaqua Nov 1, 2024
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
3 changes: 3 additions & 0 deletions app/api/[provider]/[...path]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { handle as moonshotHandler } from "../../moonshot";
import { handle as stabilityHandler } from "../../stability";
import { handle as iflytekHandler } from "../../iflytek";
import { handle as xaiHandler } from "../../xai";
import { handle as chatglmHandler } from "../../glm";
import { handle as proxyHandler } from "../../proxy";

async function handle(
Expand Down Expand Up @@ -41,6 +42,8 @@ async function handle(
return iflytekHandler(req, { params });
case ApiPath.XAI:
return xaiHandler(req, { params });
case ApiPath.ChatGLM:
return chatglmHandler(req, { params });
case ApiPath.OpenAI:
return openaiHandler(req, { params });
default:
Expand Down
3 changes: 3 additions & 0 deletions app/api/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ export function auth(req: NextRequest, modelProvider: ModelProvider) {
case ModelProvider.XAI:
systemApiKey = serverConfig.xaiApiKey;
break;
case ModelProvider.ChatGLM:
systemApiKey = serverConfig.chatglmApiKey;
break;
case ModelProvider.GPT:
default:
if (req.nextUrl.pathname.includes("azure/deployments")) {
Expand Down
129 changes: 129 additions & 0 deletions app/api/glm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { getServerSideConfig } from "@/app/config/server";
import {
CHATGLM_BASE_URL,
ApiPath,
ModelProvider,
ServiceProvider,
} from "@/app/constant";
import { prettyObject } from "@/app/utils/format";
import { NextRequest, NextResponse } from "next/server";
import { auth } from "@/app/api/auth";
import { isModelAvailableInServer } from "@/app/utils/model";

const serverConfig = getServerSideConfig();

export async function handle(
req: NextRequest,
{ params }: { params: { path: string[] } },
) {
console.log("[GLM Route] params ", params);

if (req.method === "OPTIONS") {
return NextResponse.json({ body: "OK" }, { status: 200 });
}
Comment on lines +21 to +23
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

Set CORS Headers for Preflight Requests

When handling OPTIONS requests for CORS preflight, it's important to include the appropriate headers to allow cross-origin requests. Currently, the response lacks CORS headers, which may cause client requests to fail due to CORS policy restrictions.

Apply this diff to include the necessary CORS headers:

    return NextResponse.json({ body: "OK" }, {
      status: 200,
+     headers: {
+       'Access-Control-Allow-Origin': '*', // Adjust as needed
+       'Access-Control-Allow-Methods': 'GET,POST,OPTIONS',
+       'Access-Control-Allow-Headers': 'Content-Type, Authorization',
+     },
    });
📝 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 (req.method === "OPTIONS") {
return NextResponse.json({ body: "OK" }, { status: 200 });
}
if (req.method === "OPTIONS") {
return NextResponse.json({ body: "OK" }, {
status: 200,
headers: {
'Access-Control-Allow-Origin': '*', // Adjust as needed
'Access-Control-Allow-Methods': 'GET,POST,OPTIONS',
'Access-Control-Allow-Headers': 'Content-Type, Authorization',
},
});
}


const authResult = auth(req, ModelProvider.GLM);
if (authResult.error) {
return NextResponse.json(authResult, {
status: 401,
});
}

try {
const response = await request(req);
return response;
} catch (e) {
Comment on lines +32 to +35
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

Add error cleanup in catch block.

The try-catch block should clean up any resources (like response streams) in case of an error to prevent memory leaks.

Apply this diff:

  try {
    const response = await request(req);
    return response;
  } catch (e) {
+   if (response?.body) {
+     try {
+       await response.body.cancel();
+     } catch {} // Ignore cleanup errors
+   }
    console.error("[GLM] ", e);
    return NextResponse.json(prettyObject(e));
  }

Committable suggestion skipped: line range outside the PR's diff.

console.error("[GLM] ", e);
return NextResponse.json(prettyObject(e));
}
Comment on lines +36 to +38
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

Avoid Exposing Internal Error Details to Clients

Returning the full error object in the API response may reveal sensitive internal information. It's better to return a generic error message to the client and log the detailed error on the server side.

Apply this diff to return a generic error response:

    console.error("[GLM] ", e);
-   return NextResponse.json(prettyObject(e));
+   return NextResponse.json({ error: "Internal Server Error" }, { status: 500 });
📝 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
console.error("[GLM] ", e);
return NextResponse.json(prettyObject(e));
}
console.error("[GLM] ", e);
return NextResponse.json({ error: "Internal Server Error" }, { status: 500 });
}

}

async function request(req: NextRequest) {
const controller = new AbortController();

// alibaba use base url or just remove the path
let path = `${req.nextUrl.pathname}`.replaceAll(ApiPath.ChatGLM, "");

let baseUrl = serverConfig.chatglmUrl || CHATGLM_BASE_URL;

if (!baseUrl.startsWith("http")) {
baseUrl = `https://${baseUrl}`;
}
Comment on lines +47 to +51
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

Add URL validation for security.

The base URL construction should validate the URL to prevent potential SSRF attacks.

Apply this diff:

+ function isValidUrl(url: string): boolean {
+   try {
+     const parsedUrl = new URL(url);
+     return ['http:', 'https:'].includes(parsedUrl.protocol);
+   } catch {
+     return false;
+   }
+ }

  let baseUrl = serverConfig.chatglmUrl || CHATGLM_BASE_URL;

  if (!baseUrl.startsWith("http")) {
    baseUrl = `https://${baseUrl}`;
  }
+ if (!isValidUrl(baseUrl)) {
+   throw new Error("Invalid base URL configuration");
+ }
📝 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
let baseUrl = serverConfig.chatglmUrl || CHATGLM_BASE_URL;
if (!baseUrl.startsWith("http")) {
baseUrl = `https://${baseUrl}`;
}
function isValidUrl(url: string): boolean {
try {
const parsedUrl = new URL(url);
return ['http:', 'https:'].includes(parsedUrl.protocol);
} catch {
return false;
}
}
let baseUrl = serverConfig.chatglmUrl || CHATGLM_BASE_URL;
if (!baseUrl.startsWith("http")) {
baseUrl = `https://${baseUrl}`;
}
if (!isValidUrl(baseUrl)) {
throw new Error("Invalid base URL configuration");
}


if (baseUrl.endsWith("/")) {
baseUrl = baseUrl.slice(0, -1);
}

console.log("[Proxy] ", path);
console.log("[Base Url]", baseUrl);

const timeoutId = setTimeout(
() => {
controller.abort();
},
10 * 60 * 1000,
);

const fetchUrl = `${baseUrl}${path}`;
console.log("[Fetch Url] ", fetchUrl);
const fetchOptions: RequestInit = {
headers: {
"Content-Type": "application/json",
Authorization: req.headers.get("Authorization") ?? "",
},
method: req.method,
body: req.body,
redirect: "manual",
Comment on lines +75 to +76
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

Ensure Request Body Consistency When Reading Streams

The request body (req.body) is a stream and can only be read once. Assigning it directly to fetchOptions.body and then reading it again with await req.text() can cause the stream to be empty or lead to runtime errors.

To fix this, read the body once and reuse it:

-   body: req.body,
+   const clonedBody = await req.text();
+   body: clonedBody,
...
    if (serverConfig.customModels && clonedBody) {
      try {
-       const clonedBody = await req.text();
-       fetchOptions.body = clonedBody;
+       fetchOptions.body = clonedBody;

Also applies to: 85-86

// @ts-ignore
duplex: "half",
signal: controller.signal,
};

// #1815 try to refuse some request to some models
if (serverConfig.customModels && req.body) {
try {
const clonedBody = await req.text();
fetchOptions.body = clonedBody;

const jsonBody = JSON.parse(clonedBody) as { model?: string };

// not undefined and is false
if (
isModelAvailableInServer(
serverConfig.customModels,
jsonBody?.model as string,
ServiceProvider.ChatGLM as string,
)
) {
return NextResponse.json(
{
error: true,
message: `you are not allowed to use ${jsonBody?.model} model`,
},
{
status: 403,
},
);
}
Comment on lines +91 to +107
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 inverted logic in model availability check.

The condition for returning a 403 response is incorrect. It returns 403 when the model IS available, which is the opposite of what's intended.

Apply this diff:

  if (
-   isModelAvailableInServer(
+   !isModelAvailableInServer(
      serverConfig.customModels,
      jsonBody?.model as string,
      ServiceProvider.ChatGLM as string,
    )
  ) {
📝 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 (
isModelAvailableInServer(
serverConfig.customModels,
jsonBody?.model as string,
ServiceProvider.ChatGLM as string,
)
) {
return NextResponse.json(
{
error: true,
message: `you are not allowed to use ${jsonBody?.model} model`,
},
{
status: 403,
},
);
}
if (
!isModelAvailableInServer(
serverConfig.customModels,
jsonBody?.model as string,
ServiceProvider.ChatGLM as string,
)
) {
return NextResponse.json(
{
error: true,
message: `you are not allowed to use ${jsonBody?.model} model`,
},
{
status: 403,
},
);
}

} catch (e) {
console.error(`[GLM] filter`, e);
}
}
Comment on lines +108 to +111
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

Improve error handling in model validation.

The catch block silently logs errors from JSON parsing or model validation. These should be properly handled as they indicate invalid requests.

Apply this diff:

    } catch (e) {
      console.error(`[GLM] filter`, e);
+     return NextResponse.json(
+       {
+         error: true,
+         message: "Invalid request format",
+       },
+       {
+         status: 400,
+       }
+     );
    }
📝 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
} catch (e) {
console.error(`[GLM] filter`, e);
}
}
} catch (e) {
console.error(`[GLM] filter`, e);
return NextResponse.json(
{
error: true,
message: "Invalid request format",
},
{
status: 400,
}
);
}
}

try {
const res = await fetch(fetchUrl, fetchOptions);

// to prevent browser prompt for credentials
const newHeaders = new Headers(res.headers);
newHeaders.delete("www-authenticate");
// to disable nginx buffering
newHeaders.set("X-Accel-Buffering", "no");

return new Response(res.body, {
status: res.status,
statusText: res.statusText,
headers: newHeaders,
});
} finally {
clearTimeout(timeoutId);
}
}
10 changes: 10 additions & 0 deletions app/client/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { HunyuanApi } from "./platforms/tencent";
import { MoonshotApi } from "./platforms/moonshot";
import { SparkApi } from "./platforms/iflytek";
import { XAIApi } from "./platforms/xai";
import { ChatGLMApi } from "./platforms/glm";

export const ROLES = ["system", "user", "assistant"] as const;
export type MessageRole = (typeof ROLES)[number];
Expand Down Expand Up @@ -156,6 +157,9 @@ export class ClientApi {
case ModelProvider.XAI:
this.llm = new XAIApi();
break;
case ModelProvider.ChatGLM:
this.llm = new ChatGLMApi();
break;
default:
this.llm = new ChatGPTApi();
}
Expand Down Expand Up @@ -244,6 +248,7 @@ export function getHeaders(ignoreHeaders: boolean = false) {
const isMoonshot = modelConfig.providerName === ServiceProvider.Moonshot;
const isIflytek = modelConfig.providerName === ServiceProvider.Iflytek;
const isXAI = modelConfig.providerName === ServiceProvider.XAI;
const isChatGLM = modelConfig.providerName === ServiceProvider.ChatGLM;
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

Consider refactoring the nested ternary operators

While the ChatGLM integration is functionally correct, the nested ternary operators for API key selection are becoming increasingly complex and harder to maintain. Consider refactoring this logic to improve readability.

Here's a suggested refactor using a more maintainable approach:

function getApiKeyForProvider(
  modelConfig: ModelConfig,
  accessStore: AccessStore
): string {
  const providerMap = {
    [ServiceProvider.Google]: accessStore.googleApiKey,
    [ServiceProvider.Azure]: accessStore.azureApiKey,
    [ServiceProvider.Anthropic]: accessStore.anthropicApiKey,
    [ServiceProvider.ByteDance]: accessStore.bytedanceApiKey,
    [ServiceProvider.Alibaba]: accessStore.alibabaApiKey,
    [ServiceProvider.Moonshot]: accessStore.moonshotApiKey,
    [ServiceProvider.XAI]: accessStore.xaiApiKey,
    [ServiceProvider.ChatGLM]: accessStore.chatglmApiKey,
    [ServiceProvider.Iflytek]: accessStore.iflytekApiKey && accessStore.iflytekApiSecret
      ? `${accessStore.iflytekApiKey}:${accessStore.iflytekApiSecret}`
      : "",
    // Default to OpenAI
    default: accessStore.openaiApiKey,
  };

  return providerMap[modelConfig.providerName] ?? providerMap.default;
}

This approach:

  • Improves readability and maintainability
  • Makes it easier to add new providers
  • Reduces the complexity of nested ternaries
  • Makes the provider-key mapping explicit

Also applies to: 267-268, 284-284

const isEnabledAccessControl = accessStore.enabledAccessControl();
const apiKey = isGoogle
? accessStore.googleApiKey
Expand All @@ -259,6 +264,8 @@ export function getHeaders(ignoreHeaders: boolean = false) {
? accessStore.moonshotApiKey
: isXAI
? accessStore.xaiApiKey
: isChatGLM
? accessStore.chatglmApiKey
: isIflytek
? accessStore.iflytekApiKey && accessStore.iflytekApiSecret
? accessStore.iflytekApiKey + ":" + accessStore.iflytekApiSecret
Expand All @@ -274,6 +281,7 @@ export function getHeaders(ignoreHeaders: boolean = false) {
isMoonshot,
isIflytek,
isXAI,
isChatGLM,
apiKey,
isEnabledAccessControl,
};
Expand Down Expand Up @@ -338,6 +346,8 @@ export function getClientApi(provider: ServiceProvider): ClientApi {
return new ClientApi(ModelProvider.Iflytek);
case ServiceProvider.XAI:
return new ClientApi(ModelProvider.XAI);
case ServiceProvider.ChatGLM:
return new ClientApi(ModelProvider.ChatGLM);
default:
return new ClientApi(ModelProvider.GPT);
}
Expand Down
Loading
Loading