feat(web): Integrate M4-LLM error handling improvements
Port high-value features from work/m4-llm branch into develop's security-hardened codebase: - Separate LLM vs persistence error handling in useChat (shows assistant response even when save fails) - Add structured error context logging with errorType, messagePreview, messageCount fields for debugging - Enforce state invariant in useChatOverlay: cannot be minimized when closed - Add onStorageError callback with user-friendly messages and per-error-type deduplication - Add error logging to Chat imperative handle methods - Create Chat.test.tsx with loadConversation failure mode tests Skipped from work/m4-llm (superseded by develop): - AbortSignal timeout (develop has centralized client timeout) - Custom toast system (duplicates @mosaic/ui) - ErrorBoundary (develop has its own) - WebSocket typed events (develop's ref-based pattern is superior) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -420,4 +420,175 @@ describe("useChat", () => {
|
||||
expect(result.current.error).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("error context logging", () => {
|
||||
it("should log comprehensive error context when sendMessage fails", async () => {
|
||||
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
mockSendChatMessage.mockRejectedValueOnce(new Error("LLM timeout"));
|
||||
|
||||
const { result } = renderHook(() => useChat({ model: "llama3.2" }));
|
||||
|
||||
await act(async () => {
|
||||
await result.current.sendMessage("Hello world");
|
||||
});
|
||||
|
||||
expect(consoleSpy).toHaveBeenCalledWith(
|
||||
"Failed to send chat message",
|
||||
expect.objectContaining({
|
||||
errorType: "LLM_ERROR",
|
||||
messageLength: 11,
|
||||
messagePreview: "Hello world",
|
||||
model: "llama3.2",
|
||||
timestamp: expect.any(String) as string,
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("should truncate long message previews to 50 characters", async () => {
|
||||
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
mockSendChatMessage.mockRejectedValueOnce(new Error("Failed"));
|
||||
|
||||
const longMessage = "A".repeat(100);
|
||||
const { result } = renderHook(() => useChat());
|
||||
|
||||
await act(async () => {
|
||||
await result.current.sendMessage(longMessage);
|
||||
});
|
||||
|
||||
expect(consoleSpy).toHaveBeenCalledWith(
|
||||
"Failed to send chat message",
|
||||
expect.objectContaining({
|
||||
messagePreview: "A".repeat(50),
|
||||
messageLength: 100,
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("should include message count in error context", async () => {
|
||||
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
|
||||
// First successful message
|
||||
mockSendChatMessage.mockResolvedValueOnce(createMockChatResponse("OK"));
|
||||
mockCreateConversation.mockResolvedValueOnce(createMockIdea("conv-1", "Test", ""));
|
||||
|
||||
const { result } = renderHook(() => useChat());
|
||||
|
||||
await act(async () => {
|
||||
await result.current.sendMessage("First");
|
||||
});
|
||||
|
||||
// Second message fails
|
||||
mockSendChatMessage.mockRejectedValueOnce(new Error("Fail"));
|
||||
|
||||
await act(async () => {
|
||||
await result.current.sendMessage("Second");
|
||||
});
|
||||
|
||||
// messageCount should reflect messages including the new user message
|
||||
expect(consoleSpy).toHaveBeenCalledWith(
|
||||
"Failed to send chat message",
|
||||
expect.objectContaining({
|
||||
messageCount: expect.any(Number) as number,
|
||||
})
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("LLM vs persistence error separation", () => {
|
||||
it("should show LLM error and add error message to chat when API fails", async () => {
|
||||
vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
mockSendChatMessage.mockRejectedValueOnce(new Error("Model not available"));
|
||||
|
||||
const { result } = renderHook(() => useChat());
|
||||
|
||||
await act(async () => {
|
||||
await result.current.sendMessage("Hello");
|
||||
});
|
||||
|
||||
expect(result.current.error).toBe("Model not available");
|
||||
// Should have welcome + user + error message
|
||||
expect(result.current.messages).toHaveLength(3);
|
||||
expect(result.current.messages[2]?.content).toContain("Error: Model not available");
|
||||
});
|
||||
|
||||
it("should keep assistant message visible when save fails", async () => {
|
||||
vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
mockSendChatMessage.mockResolvedValueOnce(createMockChatResponse("Great answer!"));
|
||||
mockCreateConversation.mockRejectedValueOnce(new Error("Database connection lost"));
|
||||
|
||||
const { result } = renderHook(() => useChat());
|
||||
|
||||
await act(async () => {
|
||||
await result.current.sendMessage("Hello");
|
||||
});
|
||||
|
||||
// Assistant message should still be visible
|
||||
expect(result.current.messages).toHaveLength(3); // welcome + user + assistant
|
||||
expect(result.current.messages[2]?.content).toBe("Great answer!");
|
||||
|
||||
// Error should indicate persistence failure
|
||||
expect(result.current.error).toContain("Message sent but failed to save");
|
||||
});
|
||||
|
||||
it("should log with PERSISTENCE_ERROR type when save fails", async () => {
|
||||
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
mockSendChatMessage.mockResolvedValueOnce(createMockChatResponse("Response"));
|
||||
mockCreateConversation.mockRejectedValueOnce(new Error("DB error"));
|
||||
|
||||
const { result } = renderHook(() => useChat());
|
||||
|
||||
await act(async () => {
|
||||
await result.current.sendMessage("Test");
|
||||
});
|
||||
|
||||
expect(consoleSpy).toHaveBeenCalledWith(
|
||||
"Failed to save conversation",
|
||||
expect.objectContaining({
|
||||
errorType: "PERSISTENCE_ERROR",
|
||||
})
|
||||
);
|
||||
|
||||
// Should NOT have logged as LLM_ERROR
|
||||
const llmErrorCalls = consoleSpy.mock.calls.filter((call) => {
|
||||
const ctx: unknown = call[1];
|
||||
return (
|
||||
typeof ctx === "object" &&
|
||||
ctx !== null &&
|
||||
"errorType" in ctx &&
|
||||
(ctx as { errorType: string }).errorType === "LLM_ERROR"
|
||||
);
|
||||
});
|
||||
expect(llmErrorCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("should use different user-facing messages for LLM vs save errors", async () => {
|
||||
vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
|
||||
// Test LLM error message
|
||||
mockSendChatMessage.mockRejectedValueOnce(new Error("Timeout"));
|
||||
const { result: result1 } = renderHook(() => useChat());
|
||||
|
||||
await act(async () => {
|
||||
await result1.current.sendMessage("Test");
|
||||
});
|
||||
|
||||
const llmError = result1.current.error;
|
||||
|
||||
// Test save error message
|
||||
mockSendChatMessage.mockResolvedValueOnce(createMockChatResponse("OK"));
|
||||
mockCreateConversation.mockRejectedValueOnce(new Error("DB down"));
|
||||
const { result: result2 } = renderHook(() => useChat());
|
||||
|
||||
await act(async () => {
|
||||
await result2.current.sendMessage("Test");
|
||||
});
|
||||
|
||||
const saveError = result2.current.error;
|
||||
|
||||
// They should be different
|
||||
expect(llmError).toBe("Timeout");
|
||||
expect(saveError).toContain("Message sent but failed to save");
|
||||
expect(llmError).not.toEqual(saveError);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user