-
-
Notifications
You must be signed in to change notification settings - Fork 378
fix: isolate provider endpoint pools and sticky sessions #919
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,8 +9,12 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||
| resetEndpointCircuit as resetEndpointCircuitState, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@/lib/endpoint-circuit-breaker"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { logger } from "@/lib/logger"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { PROVIDER_ENDPOINT_CONFLICT_CODE } from "@/lib/provider-endpoint-error-codes"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ENDPOINT_REFERENCED_BY_ENABLED_PROVIDERS_CODE, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| PROVIDER_ENDPOINT_CONFLICT_CODE, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@/lib/provider-endpoint-error-codes"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { probeProviderEndpointAndRecordByEndpoint } from "@/lib/provider-endpoints/probe"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { SessionManager } from "@/lib/session-manager"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ERROR_CODES } from "@/lib/utils/error-messages"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { extractZodErrorCode, formatZodError } from "@/lib/utils/zod-i18n"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -35,8 +39,9 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@/repository"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| findDashboardProviderEndpointsByVendorAndType, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| findEnabledProviderIdsByVendorAndType, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| findEnabledProviderReferencesForVendorTypeUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| findEnabledProviderVendorTypePairs, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| hasEnabledProviderReferenceForVendorTypeUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@/repository/provider-endpoints"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| findProviderEndpointProbeLogsBatch, | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -219,6 +224,25 @@ function isForeignKeyViolationError(error: unknown): boolean { | |||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| function formatProviderReferenceSummary( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| references: Array<{ id: number; name: string }>, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| maxDisplayCount: number = 3 | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const uniqueNames = Array.from( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| new Set(references.map((reference) => reference.name.trim()).filter(Boolean)) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (uniqueNames.length === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return ""; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if (uniqueNames.length <= maxDisplayCount) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return uniqueNames.join(", "); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const displayed = uniqueNames.slice(0, maxDisplayCount).join(", "); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return `${displayed} +${uniqueNames.length - maxDisplayCount}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function getProviderVendors(): Promise<ProviderVendor[]> { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const session = await getAdminSession(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -496,6 +520,21 @@ export async function editProviderEndpoint( | |||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const shouldTerminateStickySessions = | ||||||||||||||||||||||||||||||||||||||||||||||||||
| parsed.data.url !== undefined || | ||||||||||||||||||||||||||||||||||||||||||||||||||
| parsed.data.sortOrder !== undefined || | ||||||||||||||||||||||||||||||||||||||||||||||||||
| parsed.data.isEnabled !== undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (shouldTerminateStickySessions) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const affectedProviderIds = await findEnabledProviderIdsByVendorAndType( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| endpoint.vendorId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| endpoint.providerType | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| await SessionManager.terminateStickySessionsForProviders( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| affectedProviderIds, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "editProviderEndpoint" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| await publishProviderCacheInvalidation(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -551,18 +590,20 @@ export async function removeProviderEndpoint(input: unknown): Promise<ActionResu | |||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // 若该端点仍被启用 provider 引用,则不允许删除:否则会导致运行时 endpoint pool 变空/回填复活, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // 产生“删了但还在/仍被探测”的困惑(#781)。 | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const referencedByEnabledProvider = await hasEnabledProviderReferenceForVendorTypeUrl({ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const references = await findEnabledProviderReferencesForVendorTypeUrl({ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| vendorId: endpoint.vendorId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| providerType: endpoint.providerType, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| url: endpoint.url, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (referencedByEnabledProvider) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (references.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ok: false, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| error: "该端点仍被启用的供应商引用,请先修改或禁用相关供应商的 URL 后再删除", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| errorCode: ERROR_CODES.CONFLICT, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| errorCode: ENDPOINT_REFERENCED_BY_ENABLED_PROVIDERS_CODE, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| errorParams: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| count: references.length, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| providers: formatProviderReferenceSummary(references), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -585,6 +626,15 @@ export async function removeProviderEndpoint(input: unknown): Promise<ActionResu | |||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const affectedProviderIds = await findEnabledProviderIdsByVendorAndType( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| endpoint.vendorId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| endpoint.providerType | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| await SessionManager.terminateStickySessionsForProviders( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| affectedProviderIds, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "removeProviderEndpoint" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+629
to
+636
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unguarded DB call after soft-delete can produce misleading error
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/actions/provider-endpoints.ts
Line: 629-636
Comment:
**Unguarded DB call after soft-delete can produce misleading error**
`findEnabledProviderIdsByVendorAndType` is called outside any try-catch at line 629, after the endpoint has already been soft-deleted at line 610. Every other non-critical post-delete operation in this function (`resetEndpointCircuitState`, `tryDeleteProviderVendorIfEmpty`, `publishProviderCacheInvalidation`) is wrapped in its own try-catch precisely to prevent this scenario. If the DB query at line 629 throws (e.g. transient DB connectivity issue), the outer catch at line 660 returns `{ ok: false, errorCode: DELETE_FAILED }` — even though the deletion already succeeded. The UI and the caller will believe the operation failed and likely show an error, while the endpoint is actually gone.
```suggestion
try {
const affectedProviderIds = await findEnabledProviderIdsByVendorAndType(
endpoint.vendorId,
endpoint.providerType
);
await SessionManager.terminateStickySessionsForProviders(
affectedProviderIds,
"removeProviderEndpoint"
);
} catch (error) {
logger.warn("removeProviderEndpoint:terminate_sessions_failed", {
endpointId: parsed.data.endpointId,
vendorId: endpoint.vendorId,
error: error instanceof Error ? error.message : String(error),
});
}
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Auto cleanup: if the vendor has no active providers/endpoints, delete it as well. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| await tryDeleteProviderVendorIfEmpty(endpoint.vendorId); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| export const PROVIDER_ENDPOINT_CONFLICT_CODE = "PROVIDER_ENDPOINT_CONFLICT"; | ||
| export const PROVIDER_ENDPOINT_WRITE_READ_INCONSISTENCY_CODE = | ||
| "PROVIDER_ENDPOINT_WRITE_READ_INCONSISTENCY"; | ||
| export const ENDPOINT_REFERENCED_BY_ENABLED_PROVIDERS_CODE = | ||
| "ENDPOINT_REFERENCED_BY_ENABLED_PROVIDERS"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不要在 action 层拼装新的用户可见文案。
这里新增的
error中文句子,以及providers的"A, B, C +1"摘要,都会绕过现有 i18n 流程。既然已经有errorCode和errorParams,更稳妥的是只返回原始数据(例如 provider names/count),由展示层按 locale 组装文案。As per coding guidelines, "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text"
Also applies to: 593-606
🤖 Prompt for AI Agents