Skip to content

Commit 98ad239

Browse files
fforbeckfforbeck
authored
fix: properly pass the error obj to logger (#17)
1 parent 287ff27 commit 98ad239

File tree

7 files changed

+203
-5
lines changed

7 files changed

+203
-5
lines changed

‎__tests__/actions/retrieve.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
22
import { retrieveAction } from '../../src/actions/retrieve';
33
import { validateStorageClientConfig } from '../../src/environments';
44
import { defaultGatewayUrl, getCIDsFromMessage } from '../../src/utils';
5+
import { elizaLogger } from '@elizaos/core';
56

67
// Mock dependencies
78
vi.mock('@elizaos/core', () => ({
@@ -137,5 +138,71 @@ describe('retrieveAction', () => {
137138
text: expect.stringContaining('Simulated error during callback')
138139
});
139140
});
141+
142+
describe('error logging', () => {
143+
it('should log full error object with stacktrace, not just the message', async () => {
144+
const mockMessage = { content: { text: 'Get file with CID QmTest123' } };
145+
const mockCIDs = ['QmTest123'];
146+
(getCIDsFromMessage as any).mockReturnValue(mockCIDs);
147+
148+
// Create an error with a stack trace
149+
const errorWithStack = new Error('Test error with stack');
150+
151+
// Make the callback throw our error with stack
152+
mockCallback.mockImplementationOnce(() => {
153+
throw errorWithStack;
154+
});
155+
156+
// Call the handler
157+
await retrieveAction.handler(
158+
mockRuntime,
159+
mockMessage as any,
160+
{} as any,
161+
{},
162+
mockCallback
163+
);
164+
165+
// Verify error logging
166+
expect(elizaLogger.error).toHaveBeenCalled();
167+
168+
// Check that the first argument to elizaLogger.error is the full error object
169+
const errorArg = (elizaLogger.error as any).mock.calls[0][0];
170+
171+
// Verify we're passing the full error object, not just the message
172+
expect(errorArg).toBe(errorWithStack);
173+
expect(errorArg).toBeInstanceOf(Error);
174+
175+
expect(errorArg).toEqual(expect.objectContaining({
176+
message: 'Test error with stack'
177+
}));
178+
});
179+
180+
it('should include custom context message with error logs', async () => {
181+
const mockMessage = { content: { text: 'Get file with CID QmTest123' } };
182+
const mockCIDs = ['QmTest123'];
183+
(getCIDsFromMessage as any).mockReturnValue(mockCIDs);
184+
185+
const error = new Error('Test error');
186+
187+
// Make the callback throw our error
188+
mockCallback.mockImplementationOnce(() => {
189+
throw error;
190+
});
191+
192+
await retrieveAction.handler(
193+
mockRuntime,
194+
mockMessage as any,
195+
{} as any,
196+
{},
197+
mockCallback
198+
);
199+
200+
// Check that a context message is provided in the error log
201+
expect(elizaLogger.error).toHaveBeenCalledWith(
202+
error,
203+
"Error during retrieve file(s) from storage"
204+
);
205+
});
206+
});
140207
});
141208
});

‎__tests__/actions/upload.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
22
import { uploadAction } from '../../src/actions/upload';
33
import { validateStorageClientConfig } from '../../src/environments';
44
import { createStorageClient } from '../../src/clients/storage';
5+
import { elizaLogger } from '@elizaos/core';
56
import fs from 'fs';
67

78
// Mock dependencies
@@ -168,5 +169,68 @@ describe('uploadAction', () => {
168169
content: { error: 'Upload failed' }
169170
});
170171
});
172+
173+
describe('error logging', () => {
174+
it('should log full error object with stacktrace, not just the message', async () => {
175+
const mockAttachments = [
176+
{ url: 'file:///path/to/file.txt', title: 'file.txt', contentType: 'text/plain' }
177+
];
178+
const mockMessage = { content: { attachments: mockAttachments } };
179+
180+
// Create an error with a stack trace
181+
const errorWithStack = new Error('Test error with stack');
182+
183+
// Reject with the full error object that has a stack
184+
(createStorageClient as any).mockRejectedValueOnce(errorWithStack);
185+
186+
// Call the handler
187+
await uploadAction.handler(
188+
mockRuntime,
189+
mockMessage as any,
190+
{} as any,
191+
{},
192+
mockCallback
193+
);
194+
195+
// Verify error logging
196+
expect(elizaLogger.error).toHaveBeenCalled();
197+
198+
// Check that the first argument to elizaLogger.error is the full error object
199+
const errorArg = (elizaLogger.error as any).mock.calls[0][0];
200+
201+
// Verify we're passing the full error object, not just the message
202+
expect(errorArg).toBe(errorWithStack);
203+
expect(errorArg).toBeInstanceOf(Error);
204+
205+
// In a real test environment, the error would have a stack property
206+
expect(errorArg).toEqual(expect.objectContaining({
207+
message: 'Test error with stack'
208+
}));
209+
});
210+
211+
it('should include custom context message with error logs', async () => {
212+
const mockAttachments = [
213+
{ url: 'file:///path/to/file.txt', title: 'file.txt', contentType: 'text/plain' }
214+
];
215+
const mockMessage = { content: { attachments: mockAttachments } };
216+
217+
const error = new Error('Test error');
218+
(createStorageClient as any).mockRejectedValueOnce(error);
219+
220+
await uploadAction.handler(
221+
mockRuntime,
222+
mockMessage as any,
223+
{} as any,
224+
{},
225+
mockCallback
226+
);
227+
228+
// Check that a context message is provided as the second parameter
229+
expect(elizaLogger.error).toHaveBeenCalledWith(
230+
error,
231+
"Error uploading file(s) to Storacha"
232+
);
233+
});
234+
});
171235
});
172236
});

‎__tests__/clients/storage.test.ts‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,58 @@ describe('StorageClientImpl', () => {
144144
await storageClientImpl.start();
145145
expect(storageClientImpl.getStorage()).not.toBeNull();
146146

147-
await storageClientImpl.stop();
147+
await storageClientImpl.stop(mockRuntime);
148148

149149
expect(() => storageClientImpl.getStorage()).toThrow('Storage client not initialized');
150150
expect(() => storageClientImpl.getConfig()).toThrow('Storage client not initialized');
151151
});
152152

153+
describe('error logging', () => {
154+
it('should log full error object with stacktrace during client initialization', async () => {
155+
// Create an error with a stack trace
156+
const errorWithStack = new Error('API connection failed with stack');
157+
158+
// Mock the client creation to throw our error with stack
159+
const createSpy = vi.spyOn(Storage, 'create').mockRejectedValueOnce(errorWithStack);
160+
161+
// Expect the start method to throw
162+
await expect(storageClientImpl.start()).rejects.toThrow('API connection failed with stack');
163+
164+
// Verify error logging received the full error object
165+
expect(elizaLogger.error).toHaveBeenCalled();
166+
167+
// Get the first argument passed to elizaLogger.error
168+
const errorArg = (elizaLogger.error as any).mock.calls[0][0];
169+
170+
// Verify it's the full error with stack
171+
expect(errorArg).toBe(errorWithStack);
172+
expect(errorArg).toBeInstanceOf(Error);
173+
expect(errorArg).toEqual(expect.objectContaining({
174+
message: 'API connection failed with stack'
175+
}));
176+
177+
// Clean up
178+
createSpy.mockRestore();
179+
});
180+
181+
it('should include custom context message with error logs', async () => {
182+
const errorWithStack = new Error('Client initialization failed');
183+
const createSpy = vi.spyOn(Storage, 'create').mockRejectedValueOnce(errorWithStack);
184+
185+
// Try to start the client, which should fail
186+
await expect(storageClientImpl.start()).rejects.toThrow('Client initialization failed');
187+
188+
// Check that a context message is provided as the second parameter
189+
expect(elizaLogger.error).toHaveBeenCalledWith(
190+
errorWithStack,
191+
"❌ Storage client failed to start"
192+
);
193+
194+
// Clean up
195+
createSpy.mockRestore();
196+
});
197+
});
198+
153199
describe('getContent', () => {
154200
it('should fetch content from the configured gateway URL', async () => {
155201
await storageClientImpl.start();
@@ -183,6 +229,27 @@ describe('StorageClientImpl', () => {
183229

184230
await expect(storageClientImpl.getContent(testCid)).rejects.toThrow('Network error');
185231
});
232+
233+
it('should log full error object when fetch fails', async () => {
234+
await storageClientImpl.start();
235+
const testCid = 'bafytest123';
236+
const fetchError = new Error('Network fetch error with stack');
237+
(global.fetch as any).mockRejectedValueOnce(fetchError);
238+
239+
// The getContent method doesn't have error logging, so this test is confirming
240+
// that it properly propagates the error without losing the stack
241+
await expect(storageClientImpl.getContent(testCid)).rejects.toThrow('Network fetch error with stack');
242+
243+
// Verify the error object is intact with its properties
244+
try {
245+
await storageClientImpl.getContent(testCid);
246+
} catch (error) {
247+
expect(error).toBe(fetchError);
248+
expect(error).toBeInstanceOf(Error);
249+
expect(error).toHaveProperty('stack');
250+
expect(error.message).toBe('Network fetch error with stack');
251+
}
252+
});
186253
});
187254
});
188255

‎src/actions/retrieve.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export const retrieveAction: Action = {
7272
elizaLogger.log("File(s) retrieved successfully!");
7373
return true;
7474
} catch (error) {
75-
elizaLogger.error("Error during retrieve file(s) from storage:", error);
75+
elizaLogger.error(error, "Error during retrieve file(s) from storage");
7676
await callback?.({ text: `Error during retrieve file(s) from storage: ${error.message}` });
7777
return false;
7878
}

‎src/actions/upload.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export const uploadAction: Action = {
8181
elizaLogger.success("File(s) uploaded to Storacha");
8282
return true;
8383
} catch (error) {
84-
elizaLogger.error("Error uploading file(s) to Storacha", error);
84+
elizaLogger.error(error, "Error uploading file(s) to Storacha");
8585
await callback?.({
8686
text: "I'm sorry, I couldn't upload the file(s) to Storacha. Please try again later.",
8787
content: { error: error.message },

‎src/clients/storage.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export class StorageClientInstanceImpl implements ClientInstance {
2424
this.storage = await createStorageClient(this.config);
2525
elizaLogger.success(`✅ Storage client successfully started`);
2626
} catch (error) {
27-
elizaLogger.error(`❌ Storage client failed to start: ${error}`);
27+
elizaLogger.error(error, "❌ Storage client failed to start");
2828
throw error;
2929
}
3030
}

‎src/environments.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export async function validateStorageClientConfig(
3838
const c = storageClientEnvSchema.parse(config);
3939
return c;
4040
} catch (error) {
41-
elizaLogger.error("Storage client config validation failed", error);
41+
elizaLogger.error(error, "Storage client config validation failed");
4242
if (error instanceof z.ZodError) {
4343
const errorMessages = error.errors
4444
.map((err) => `${err.path.join(".")}: ${err.message}`)

0 commit comments

Comments
 (0)