mirror of
https://github.com/lobehub/lobe-chat.git
synced 2026-06-13 19:20:04 +00:00
🐛 fix(agent-runtime): classify topic/agent/session FK violations as ConversationParentMissing (#15408)
When a user deletes a topic (or agent/session/thread) while an agent operation is still running, the assistant/tool-message INSERT fails with a Postgres 23503 foreign_key_violation on the corresponding `messages` FK. The persist-error guard only recognised the `messages_parent_id_messages_id_fk` self-FK, so every other reference deletion slipped through as a raw `Failed query: insert into "messages"` 500 — surfacing to the user as a driver/SQL error and polluting the error dashboard as DatabasePersistError noise (one of the longest-standing top error categories). Generalise `isParentMessageMissingError` → `isMidOperationReferenceMissingError` to match a 23503 violation on any of the mid-operation-deletable `messages` references (parent / quota message, topic, agent, session, thread). These all mean "the referenced context was deleted mid-flight" — a lost race against the user, not a runtime failure — so they are normalised to the typed, user-side `ConversationParentMissing` error like the parent case already was. Out-of-scope FKs (e.g. `messages_user_id_users_id_fk`, other tables) stay real failures. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -96,7 +96,7 @@ import { formatErrorEventData } from './formatErrorEventData';
|
||||
import { classifyLLMError, type LLMErrorKind } from './llmErrorClassification';
|
||||
import {
|
||||
createConversationParentMissingError,
|
||||
isParentMessageMissingError,
|
||||
isMidOperationReferenceMissingError,
|
||||
isPersistFatal,
|
||||
markPersistFatal,
|
||||
} from './messagePersistErrors';
|
||||
@@ -2189,7 +2189,7 @@ export const createRuntimeExecutors = (
|
||||
// Normalize BEFORE publishing so clients (which treat `error` stream
|
||||
// events as terminal and surface `event.data.error` directly) see the
|
||||
// typed business error, not the raw SQL / driver text.
|
||||
const fatal = isParentMessageMissingError(error)
|
||||
const fatal = isMidOperationReferenceMissingError(error)
|
||||
? createConversationParentMissingError(payload.parentMessageId, error)
|
||||
: error instanceof Error
|
||||
? error
|
||||
@@ -2696,7 +2696,7 @@ export const createRuntimeExecutors = (
|
||||
// events as terminal and surface `event.data.error` directly, so
|
||||
// a raw SQL error here would leak driver text to the user before
|
||||
// the ConversationParentMissing throw is consumed. See .
|
||||
const fatal = isParentMessageMissingError(error)
|
||||
const fatal = isMidOperationReferenceMissingError(error)
|
||||
? createConversationParentMissingError(parentMessageId, error)
|
||||
: error instanceof Error
|
||||
? error
|
||||
@@ -3440,7 +3440,7 @@ export const createRuntimeExecutors = (
|
||||
);
|
||||
// Normalize BEFORE publishing so clients surface the typed business
|
||||
// error instead of the raw driver text (see review).
|
||||
const fatal = isParentMessageMissingError(error)
|
||||
const fatal = isMidOperationReferenceMissingError(error)
|
||||
? createConversationParentMissingError(parentMessageId, error)
|
||||
: error instanceof Error
|
||||
? error
|
||||
|
||||
@@ -2,42 +2,61 @@ import { describe, expect, it } from 'vitest';
|
||||
|
||||
import {
|
||||
createConversationParentMissingError,
|
||||
isParentMessageMissingError,
|
||||
isMidOperationReferenceMissingError,
|
||||
isPersistFatal,
|
||||
markPersistFatal,
|
||||
} from '../messagePersistErrors';
|
||||
|
||||
describe('isParentMessageMissingError', () => {
|
||||
describe('isMidOperationReferenceMissingError', () => {
|
||||
it('matches the drizzle + postgres-js error shape (FK via .cause)', () => {
|
||||
const error: any = new Error('Failed query: insert into messages ...');
|
||||
error.cause = { code: '23503', constraint: 'messages_parent_id_messages_id_fk' };
|
||||
expect(isParentMessageMissingError(error)).toBe(true);
|
||||
expect(isMidOperationReferenceMissingError(error)).toBe(true);
|
||||
});
|
||||
|
||||
it('matches top-level code/constraint_name variants', () => {
|
||||
const error: any = new Error('x');
|
||||
error.code = '23503';
|
||||
error.constraint_name = 'messages_parent_id_messages_id_fk';
|
||||
expect(isParentMessageMissingError(error)).toBe(true);
|
||||
expect(isMidOperationReferenceMissingError(error)).toBe(true);
|
||||
});
|
||||
|
||||
it('does not match other FK violations (different constraint)', () => {
|
||||
it.each([
|
||||
'messages_parent_id_messages_id_fk',
|
||||
'messages_quota_id_messages_id_fk',
|
||||
'messages_topic_id_topics_id_fk',
|
||||
'messages_agent_id_agents_id_fk',
|
||||
'messages_session_id_sessions_id_fk',
|
||||
'messages_thread_id_threads_id_fk',
|
||||
])('matches every mid-operation-deletable reference FK: %s', (constraint) => {
|
||||
const error: any = new Error('x');
|
||||
error.cause = { code: '23503', constraint: 'messages_topic_id_topics_id_fk' };
|
||||
expect(isParentMessageMissingError(error)).toBe(false);
|
||||
error.cause = { code: '23503', constraint };
|
||||
expect(isMidOperationReferenceMissingError(error)).toBe(true);
|
||||
});
|
||||
|
||||
it('does not match FK violations on out-of-scope constraints', () => {
|
||||
// user-account deletion / non-messages tables stay real failures
|
||||
const userFk: any = new Error('x');
|
||||
userFk.cause = { code: '23503', constraint: 'messages_user_id_users_id_fk' };
|
||||
expect(isMidOperationReferenceMissingError(userFk)).toBe(false);
|
||||
|
||||
const otherTable: any = new Error('x');
|
||||
otherTable.cause = { code: '23503', constraint: 'files_user_id_users_id_fk' };
|
||||
expect(isMidOperationReferenceMissingError(otherTable)).toBe(false);
|
||||
});
|
||||
|
||||
it('does not match non-FK pg errors', () => {
|
||||
const error: any = new Error('x');
|
||||
error.cause = { code: '23505', constraint: 'messages_parent_id_messages_id_fk' };
|
||||
expect(isParentMessageMissingError(error)).toBe(false);
|
||||
expect(isMidOperationReferenceMissingError(error)).toBe(false);
|
||||
});
|
||||
|
||||
it('handles null / non-object safely', () => {
|
||||
expect(isParentMessageMissingError(null)).toBe(false);
|
||||
expect(isParentMessageMissingError(undefined)).toBe(false);
|
||||
expect(isParentMessageMissingError('string-error')).toBe(false);
|
||||
expect(isParentMessageMissingError(42)).toBe(false);
|
||||
it('handles null / non-object / missing-constraint safely', () => {
|
||||
expect(isMidOperationReferenceMissingError(null)).toBe(false);
|
||||
expect(isMidOperationReferenceMissingError(undefined)).toBe(false);
|
||||
expect(isMidOperationReferenceMissingError('string-error')).toBe(false);
|
||||
expect(isMidOperationReferenceMissingError(42)).toBe(false);
|
||||
expect(isMidOperationReferenceMissingError({ code: '23503' })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -8,11 +8,25 @@ import { AgentRuntimeErrorType } from '@lobechat/types';
|
||||
const PG_FOREIGN_KEY_VIOLATION = '23503';
|
||||
|
||||
/**
|
||||
* Constraint name drizzle generates for the `messages.parent_id` self-FK.
|
||||
* Hard-coded because we only use it as a signature — no need to reflect over
|
||||
* the schema at runtime.
|
||||
* Constraint names drizzle generates for the `messages` foreign keys that point
|
||||
* at rows a user can delete *while an operation is still running* — the parent /
|
||||
* quota message, the topic, the agent, the session, the thread. When any of
|
||||
* these rows is removed mid-flight, the assistant/tool-message INSERT fails with
|
||||
* a 23503 foreign_key_violation. That's a lost race against the user, not a
|
||||
* runtime bug.
|
||||
*
|
||||
* Hard-coded because we only use them as signatures — no need to reflect over
|
||||
* the schema at runtime. An entry for a constraint that doesn't exist is a
|
||||
* harmless no-op.
|
||||
*/
|
||||
const MESSAGES_PARENT_FK_CONSTRAINT = 'messages_parent_id_messages_id_fk';
|
||||
const MID_OPERATION_DELETABLE_FK_CONSTRAINTS = new Set([
|
||||
'messages_parent_id_messages_id_fk', // parent message deleted
|
||||
'messages_quota_id_messages_id_fk', // quota (root) message deleted
|
||||
'messages_topic_id_topics_id_fk', // topic deleted
|
||||
'messages_agent_id_agents_id_fk', // agent deleted
|
||||
'messages_session_id_sessions_id_fk', // session deleted
|
||||
'messages_thread_id_threads_id_fk', // thread deleted
|
||||
]);
|
||||
|
||||
/**
|
||||
* Internal property the runtime uses to mark a thrown error as coming from
|
||||
@@ -23,15 +37,21 @@ const MESSAGES_PARENT_FK_CONSTRAINT = 'messages_parent_id_messages_id_fk';
|
||||
export const PERSIST_FATAL_MARKER = 'persistFatal';
|
||||
|
||||
/**
|
||||
* Detect whether an error returned by `messageModel.create` is a `parent_id`
|
||||
* FK violation — meaning the parent message no longer exists. Most commonly
|
||||
* caused by the parent being deleted concurrently with agent execution
|
||||
* (see ).
|
||||
* Detect whether an error returned by `messageModel.create` is a foreign-key
|
||||
* violation on one of the mid-operation-deletable `messages` references — the
|
||||
* parent / quota message, topic, agent, session or thread no longer exists,
|
||||
* almost always because the user deleted that context concurrently with agent
|
||||
* execution.
|
||||
*
|
||||
* Previously this only matched the `parent_id` self-FK, so topic / agent / etc.
|
||||
* deletions slipped through as raw `Failed query: insert into "messages"` 500s
|
||||
* (DatabasePersistError noise on the dashboard) instead of the typed user-side
|
||||
* error.
|
||||
*
|
||||
* `drizzle` + `postgres-js` wrap the raw PG error as `.cause`, so the check
|
||||
* looks at both the top level and the cause.
|
||||
*/
|
||||
export const isParentMessageMissingError = (error: unknown): boolean => {
|
||||
export const isMidOperationReferenceMissingError = (error: unknown): boolean => {
|
||||
if (!error || typeof error !== 'object') return false;
|
||||
const err = error as any;
|
||||
const code = err?.code ?? err?.cause?.code;
|
||||
@@ -40,7 +60,11 @@ export const isParentMessageMissingError = (error: unknown): boolean => {
|
||||
err?.constraint ??
|
||||
err?.cause?.constraint_name ??
|
||||
err?.cause?.constraint;
|
||||
return code === PG_FOREIGN_KEY_VIOLATION && constraint === MESSAGES_PARENT_FK_CONSTRAINT;
|
||||
return (
|
||||
code === PG_FOREIGN_KEY_VIOLATION &&
|
||||
typeof constraint === 'string' &&
|
||||
MID_OPERATION_DELETABLE_FK_CONSTRAINTS.has(constraint)
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user