-
Notifications
You must be signed in to change notification settings - Fork 9
fix: logger compatibility #38
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 |
|---|---|---|
|
|
@@ -60,17 +60,17 @@ export class KnowledgeService extends Service { | |
| try { | ||
| // Use a small delay to ensure runtime is fully ready if needed, though constructor implies it should be. | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)); | ||
|
|
||
| // Get the agent-specific knowledge path from runtime settings | ||
| const knowledgePath = this.runtime.getSetting('KNOWLEDGE_PATH'); | ||
|
|
||
| const result: LoadResult = await loadDocsFromPath( | ||
| this as any, | ||
| this as any, | ||
| this.runtime.agentId, | ||
| undefined, // worldId | ||
| knowledgePath | ||
| ); | ||
|
Comment on lines
67
to
72
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. 💡 Verification agent 🧩 Analysis chainDrop the Passing Proposed change here: - const result: LoadResult = await loadDocsFromPath(
- this as any,
+ const result: LoadResult = await loadDocsFromPath(
+ this,
this.runtime.agentId,
undefined, // worldId
knowledgePath
);And in src/docs-loader.ts (outside this diff), ensure the KnowledgeService type is imported type-only to avoid runtime cycles: // at top of docs-loader.ts
import type { KnowledgeService } from './service.ts';Alternatively, decouple the function signature to accept a narrow interface and avoid referencing the class type: // docs-loader.ts
export type KnowledgeAdder = (opts: AddKnowledgeOptions) => Promise<{ fragmentCount: number }>;
export async function loadDocsFromPath(
service: { addKnowledge: KnowledgeAdder },
agentId: UUID,
worldId?: UUID,
knowledgePath?: string
) { /* ... */ }Please confirm docs-loader.ts uses a type-only import or a narrowed interface so we can safely remove the I’ll check how loadDocsFromPath is defined and whether docs-loader.ts already uses a type-only import. 🏁 Script executed: #!/bin/bash
set -e
# Show the declaration of loadDocsFromPath in docs-loader.ts
echo "=== loadDocsFromPath Declaration ==="
rg --context 3 --max-count 1 "export async function loadDocsFromPath" -n src/docs-loader.ts
# Show the top of docs-loader.ts to inspect imports
echo
echo "=== Top of docs-loader.ts ==="
head -n 20 src/docs-loader.tsLength of output: 1322 Use a type-only import in docs-loader and remove the To preserve strong typing and avoid runtime cycles, update both files: • src/service.ts (around the Proposed diffs: --- a/src/service.ts
+++ b/src/service.ts
@@ -67,7 +67,7 @@
const result: LoadResult = await loadDocsFromPath(
- this as any,
+ this,
this.runtime.agentId,
undefined, // worldId
knowledgePath
);--- a/src/docs-loader.ts
+++ b/src/docs-loader.ts
@@ -3,7 +3,7 @@
import { logger, UUID } from '@elizaos/core';
import * as fs from 'fs';
import * as path from 'path';
-import { KnowledgeService } from './service.ts';
+import type { KnowledgeService } from './service.ts';
import { AddKnowledgeOptions } from './types.ts';
import { isBinaryContentType } from './utils.ts';These changes let you drop the 🤖 Prompt for AI Agents |
||
|
|
||
| if (result.successful > 0) { | ||
| logger.info( | ||
| `KnowledgeService: Loaded ${result.successful} documents from docs folder on startup for agent ${this.runtime.agentId}` | ||
|
|
@@ -82,8 +82,8 @@ export class KnowledgeService extends Service { | |
| } | ||
| } catch (error) { | ||
| logger.error( | ||
| `KnowledgeService: Error loading documents on startup for agent ${this.runtime.agentId}:`, | ||
| error | ||
| { error }, | ||
| `KnowledgeService: Error loading documents on startup for agent ${this.runtime.agentId}` | ||
| ); | ||
| } | ||
| } | ||
|
|
@@ -161,14 +161,14 @@ export class KnowledgeService extends Service { | |
| } | ||
| } | ||
|
|
||
| logger.success('Model configuration validated successfully.'); | ||
| logger.success(`Knowledge Plugin initialized for agent: ${runtime.character.name}`); | ||
| logger.info('Model configuration validated successfully.'); | ||
| logger.info(`Knowledge Plugin initialized for agent: ${runtime.character.name}`); | ||
|
|
||
| logger.info( | ||
| 'Knowledge Plugin initialized. Frontend panel should be discoverable via its public route.' | ||
| ); | ||
| } catch (error) { | ||
| logger.error('Failed to initialize Knowledge plugin:', error); | ||
| logger.error({ error }, 'Failed to initialize Knowledge plugin'); | ||
| throw error; | ||
| } | ||
|
|
||
|
|
@@ -178,7 +178,7 @@ export class KnowledgeService extends Service { | |
| if (service.config.LOAD_DOCS_ON_STARTUP) { | ||
| logger.info('LOAD_DOCS_ON_STARTUP is enabled. Loading documents from docs folder...'); | ||
| service.loadInitialDocuments().catch((error) => { | ||
| logger.error('Error during initial document loading in KnowledgeService:', error); | ||
| logger.error({ error }, 'Error during initial document loading in KnowledgeService'); | ||
| }); | ||
| } else { | ||
| logger.info('LOAD_DOCS_ON_STARTUP is disabled. Skipping automatic document loading.'); | ||
|
|
@@ -195,8 +195,8 @@ export class KnowledgeService extends Service { | |
| // Run in background, don't await here to prevent blocking startup | ||
| await service.processCharacterKnowledge(stringKnowledge).catch((err) => { | ||
| logger.error( | ||
| `KnowledgeService: Error processing character knowledge during startup: ${err.message}`, | ||
| err | ||
| { error: err }, | ||
| 'KnowledgeService: Error processing character knowledge during startup' | ||
| ); | ||
| }); | ||
| } else { | ||
|
|
@@ -330,7 +330,8 @@ export class KnowledgeService extends Service { | |
| fileBuffer = Buffer.from(content, 'base64'); | ||
| } catch (e: any) { | ||
| logger.error( | ||
| `KnowledgeService: Failed to convert base64 to buffer for ${originalFilename}: ${e.message}` | ||
| { error: e }, | ||
| `KnowledgeService: Failed to convert base64 to buffer for ${originalFilename}` | ||
| ); | ||
| throw new Error(`Invalid base64 content for PDF file ${originalFilename}`); | ||
| } | ||
|
|
@@ -342,7 +343,8 @@ export class KnowledgeService extends Service { | |
| fileBuffer = Buffer.from(content, 'base64'); | ||
| } catch (e: any) { | ||
| logger.error( | ||
| `KnowledgeService: Failed to convert base64 to buffer for ${originalFilename}: ${e.message}` | ||
| { error: e }, | ||
| `KnowledgeService: Failed to convert base64 to buffer for ${originalFilename}` | ||
| ); | ||
| throw new Error(`Invalid base64 content for binary file ${originalFilename}`); | ||
| } | ||
|
|
@@ -373,9 +375,7 @@ export class KnowledgeService extends Service { | |
| extractedText = decodedText; | ||
| documentContentToStore = decodedText; | ||
| } catch (e) { | ||
| logger.error( | ||
| `Failed to decode base64 for ${originalFilename}: ${e instanceof Error ? e.message : String(e)}` | ||
| ); | ||
| logger.error({ error: e as any }, `Failed to decode base64 for ${originalFilename}`); | ||
| // If it looked like base64 but failed to decode properly, this is an error | ||
| throw new Error( | ||
| `File ${originalFilename} appears to be corrupted or incorrectly encoded` | ||
|
|
@@ -452,8 +452,8 @@ export class KnowledgeService extends Service { | |
| }; | ||
| } catch (error: any) { | ||
| logger.error( | ||
| `KnowledgeService: Error processing document ${originalFilename}: ${error.message}`, | ||
| error.stack | ||
| { error, stack: error.stack }, | ||
| `KnowledgeService: Error processing document ${originalFilename}` | ||
| ); | ||
| throw error; | ||
| } | ||
|
|
@@ -462,7 +462,7 @@ export class KnowledgeService extends Service { | |
| // --- Knowledge methods moved from AgentRuntime --- | ||
|
|
||
| private async handleProcessingError(error: any, context: string) { | ||
| logger.error(`KnowledgeService: Error ${context}:`, error?.message || error || 'Unknown error'); | ||
| logger.error({ error }, `KnowledgeService: Error ${context}`); | ||
| throw error; | ||
| } | ||
|
|
||
|
|
@@ -797,8 +797,8 @@ export class KnowledgeService extends Service { | |
| fragmentsProcessed++; | ||
| } catch (error) { | ||
| logger.error( | ||
| `KnowledgeService: Error processing fragment ${fragment.id} for document ${item.id}:`, | ||
| error | ||
| { error }, | ||
| `KnowledgeService: Error processing fragment ${fragment.id} for document ${item.id}` | ||
| ); | ||
| } | ||
| } | ||
|
|
@@ -816,10 +816,7 @@ export class KnowledgeService extends Service { | |
| // Store the fragment in the knowledge table | ||
| await this.runtime.createMemory(fragment, 'knowledge'); | ||
| } catch (error) { | ||
| logger.error( | ||
| `KnowledgeService: Error processing fragment ${fragment.id}:`, | ||
| error instanceof Error ? error.message : String(error) | ||
| ); | ||
| logger.error({ error }, `KnowledgeService: Error processing fragment ${fragment.id}`); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -555,7 +555,7 @@ export class KnowledgeTestSuite implements TestSuite { | |
| } | ||
|
|
||
| // Verify service is registered | ||
| runtime.services.set(KnowledgeService.serviceType as any, service); | ||
| runtime.services.set(KnowledgeService.serviceType as any, [service]); | ||
| const retrievedService = runtime.getService(KnowledgeService.serviceType); | ||
|
|
||
|
Comment on lines
+558
to
560
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. 💡 Verification agent 🧩 Analysis chainArray-based service registry requires mock runtime updates You’re storing Update the mock to hold arrays and make - const services: Map<string, Service> = new Map();
+ const services: Map<string, Service[]> = new Map();- getService<T extends Service>(name: string): T | null {
- return (services.get(name) as T) || null;
- },
+ getService<T extends Service>(name: string): T | null {
+ const entry = services.get(name);
+ return (entry?.[0] as T) || null;
+ },- getAllServices() {
- return services;
- },
+ getAllServices() {
+ return services; // Map<string, Service[]>
+ },- async registerService(ServiceClass: typeof Service) {
- const service = await ServiceClass.start(this);
- services.set(ServiceClass.serviceType, service);
- },
+ async registerService(ServiceClass: typeof Service) {
+ const service = await ServiceClass.start(this);
+ const key = ServiceClass.serviceType;
+ const list = services.get(key) ?? [];
+ list.push(service);
+ services.set(key, list);
+ },After applying the above, the equality check in this test should succeed and the other tests that do Let’s locate the mock runtime implementation to confirm its current shape before making refactors. 🏁 Script executed: #!/bin/bash
# Find where the services Map is declared
rg -n "const services" -A3
# Find getService definition
rg -n "getService<.*>" -A5
# Find getAllServices
rg -n "getAllServices" -A3
# Find registerService implementation
rg -n "registerService" -A5Length of output: 3500 Refactor mock runtime to support array-based service registry The mock in • Change the --- a/src/tests.ts
+++ b/src/tests.ts
@@ 92c92
- const services: Map<string, Service> = new Map();
+ const services: Map<string, Service[]> = new Map();
@@ 331,333c331,335
- getService<T extends Service>(name: string): T | null {
- return (services.get(name) as T) || null;
- },
+ getService<T extends Service>(name: string): T | null {
+ const entry = services.get(name);
+ return (entry?.[0] as T) || null;
+ },
@@ 335,337c337,338
- getAllServices() {
- return services;
- },
+ getAllServices(): Map<string, Service[]> {
+ return services;
+ },
@@ 339,342c340,345
- async registerService(ServiceClass: typeof Service) {
- const service = await ServiceClass.start(this);
- services.set(ServiceClass.serviceType, service);
- },
+ async registerService(ServiceClass: typeof Service) {
+ const service = await ServiceClass.start(this);
+ const key = ServiceClass.serviceType;
+ const list = services.get(key) ?? [];
+ list.push(service);
+ services.set(key, list);
+ },After applying these changes, your lifecycle test’s equality check (
🤖 Prompt for AI Agents |
||
| if (retrievedService !== service) { | ||
|
|
@@ -635,7 +635,7 @@ export class KnowledgeTestSuite implements TestSuite { | |
| name: 'Should add knowledge successfully', | ||
| fn: async (runtime: IAgentRuntime) => { | ||
| const service = await KnowledgeService.start(runtime); | ||
| runtime.services.set(KnowledgeService.serviceType as any, service); | ||
| runtime.services.set(KnowledgeService.serviceType as any, [service]); | ||
|
|
||
| const testDocument = { | ||
| clientDocumentId: uuidv4() as UUID, | ||
|
|
@@ -675,7 +675,7 @@ export class KnowledgeTestSuite implements TestSuite { | |
| name: 'Should handle duplicate document uploads', | ||
| fn: async (runtime: IAgentRuntime) => { | ||
| const service = await KnowledgeService.start(runtime); | ||
| runtime.services.set(KnowledgeService.serviceType as any, service); | ||
| runtime.services.set(KnowledgeService.serviceType as any, [service]); | ||
|
|
||
| const testDocument = { | ||
| clientDocumentId: uuidv4() as UUID, | ||
|
|
@@ -711,7 +711,7 @@ export class KnowledgeTestSuite implements TestSuite { | |
| name: 'Should retrieve knowledge based on query', | ||
| fn: async (runtime: IAgentRuntime) => { | ||
| const service = await KnowledgeService.start(runtime); | ||
| runtime.services.set(KnowledgeService.serviceType as any, service); | ||
| runtime.services.set(KnowledgeService.serviceType as any, [service]); | ||
|
|
||
| // Add some test knowledge | ||
| const testDocument = { | ||
|
|
@@ -762,7 +762,7 @@ export class KnowledgeTestSuite implements TestSuite { | |
| name: 'Should format knowledge in provider output', | ||
| fn: async (runtime: IAgentRuntime) => { | ||
| const service = await KnowledgeService.start(runtime); | ||
| runtime.services.set('knowledge' as any, service); | ||
| runtime.services.set('knowledge' as any, [service]); | ||
|
|
||
| // Add test knowledge | ||
| const testDocument = { | ||
|
|
@@ -885,7 +885,7 @@ export class KnowledgeTestSuite implements TestSuite { | |
| name: 'Should handle and log errors appropriately', | ||
| fn: async (runtime: IAgentRuntime) => { | ||
| const service = await KnowledgeService.start(runtime); | ||
| runtime.services.set(KnowledgeService.serviceType as any, service); | ||
| runtime.services.set(KnowledgeService.serviceType as any, [service]); | ||
|
|
||
| // Clear previous mock calls | ||
| mockLogger.clearCalls(); | ||
|
|
@@ -952,8 +952,8 @@ export class KnowledgeTestSuite implements TestSuite { | |
|
|
||
| // Start service | ||
| const service = await KnowledgeService.start(runtime); | ||
| runtime.services.set(KnowledgeService.serviceType as any, service); | ||
| runtime.services.set('knowledge' as any, service); | ||
| runtime.services.set(KnowledgeService.serviceType as any, [service]); | ||
| runtime.services.set('knowledge' as any, [service]); | ||
|
|
||
| // Register provider | ||
| runtime.registerProvider(knowledgeProvider); | ||
|
|
@@ -1029,7 +1029,7 @@ export class KnowledgeTestSuite implements TestSuite { | |
| name: 'Should handle large documents with chunking', | ||
| fn: async (runtime: IAgentRuntime) => { | ||
| const service = await KnowledgeService.start(runtime); | ||
| runtime.services.set(KnowledgeService.serviceType as any, service); | ||
| runtime.services.set(KnowledgeService.serviceType as any, [service]); | ||
|
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. Bug: Service Retrieval Returns Array Instead of ObjectServices are incorrectly stored as arrays in the |
||
|
|
||
| // Create a large document | ||
| const largeContent = Array(100) | ||
|
|
||
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.
💡 Verification agent
🧩 Analysis chain
Structured error logging adopted here
Good switch to structured error logging. Confirm that the logger recognizes the "error" key for proper stack capture (some loggers expect an Error or
{ err }).Run this quick check to spot non-structured usages still present and ensure consistency:
🏁 Script executed:
Length of output: 2720
🏁 Script executed:
Length of output: 4824
Ensure consistent structured error logging
We still have a few
logger.errorcalls using the old(message, errorVar)signature, which won’t capture stacks under our new structured‐logging setup. Please convert these to pass the error object under anerrorkey:• src/routes.ts:324
• src/utils.ts:172
After updating these, run a global check (e.g.
rg "logger\\.error" -g "src/**" | grep -v "{ error") to catch any remaining unstructured calls. Also verify your logger implementation picks up theerrorkey for stack traces.🤖 Prompt for AI Agents