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
+462 −1
Merged

feat: [#5714] 支持GLM #5741

Show file tree
Hide file tree
Changes from 1 commit
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 glmHandler } 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.GLM:
return glmHandler(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.GLM:
systemApiKey = serverConfig.glmApiKey;
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

GLM configuration is properly implemented but needs documentation

The implementation of GLM API key is correctly structured in the codebase:

  • Proper environment variable handling in server.ts
  • Type definitions and validation through isGLM check
  • Consistent integration with the authentication system

However, the GLM configuration is missing from the documentation. The developer should:

  • Add GLM_API_KEY to the environment variables section in README.md
  • Include GLM_URL configuration instructions
  • Document the GLM provider setup process
🔗 Analysis chain

LGTM! Verify server configuration for GLM.

The implementation follows the established pattern for model providers and correctly assigns the GLM API key.

Let's verify the server configuration includes the GLM API key definition:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify GLM API key configuration in server config
# Expected: Find the definition of glmApiKey in server configuration

# Search for GLM API key configuration
rg -l 'glmApiKey'

# Check the type definition to ensure GLM API key is properly typed
ast-grep --pattern 'interface $_ {
  $$$
  glmApiKey?: $_
  $$$
}'

Length of output: 189

Script:

#!/bin/bash
# Let's check the server config implementation and type definitions
rg -A 5 -B 5 'glmApiKey' app/config/server.ts

# Also check the interface/type definitions that might contain glmApiKey
ast-grep --pattern 'type $_ = {
  $$$
  glmApiKey?: $_
  $$$
}'

# Check if there are any validation or requirement checks for glmApiKey
rg -A 3 'glmApiKey.*required|required.*glmApiKey|validate.*glmApiKey'

Length of output: 615

Script:

#!/bin/bash
# Let's check the isGLM configuration and usage
rg -A 2 -B 2 'isGLM'

# Check if there's any validation or initialization of GLM_API_KEY env var
rg -A 2 'GLM_API_KEY'

# Look for any GLM-related environment variable documentation
rg -l '\.env|environment|config' --type md

Length of output: 2251

Script:

#!/bin/bash
# Check README and documentation for GLM configuration instructions
rg -A 5 -B 5 'GLM|Environment' README.md

# Check environment variable declarations in any example env files
fd -e example -e sample -e template | grep -i env | xargs cat

# Look for any GLM-related configuration in the docs
rg -A 3 'GLM' docs/

Length of output: 368

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 {
GLM_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.GLM, "");

let baseUrl = serverConfig.glmUrl || GLM_BASE_URL;

if (!baseUrl.startsWith("http")) {
baseUrl = `https://${baseUrl}`;
}

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.GLM 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 { GLMApi } 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.GLM:
this.llm = new GLMApi();
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 isGLM = modelConfig.providerName === ServiceProvider.GLM;
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
: isGLM
? accessStore.glmApiKey
: isIflytek
? accessStore.iflytekApiKey && accessStore.iflytekApiSecret
? accessStore.iflytekApiKey + ":" + accessStore.iflytekApiSecret
Expand All @@ -274,6 +281,7 @@ export function getHeaders(ignoreHeaders: boolean = false) {
isMoonshot,
isIflytek,
isXAI,
isGLM,
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.GLM:
return new ClientApi(ModelProvider.GLM);
default:
return new ClientApi(ModelProvider.GPT);
}
Expand Down
Loading
Loading