Skip to content

fix: handle circular error references in logger and handlers (GIT-109) #75

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.

Sign up for GitHub

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

Merged
TonyCasey merged 1 commit into from
Feb 15, 2026
Merged

fix: handle circular error references in logger and handlers (GIT-109) #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/application/handlers/CommitMsgHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,15 @@ export class CommitMsgHandler implements IEventHandler<ICommitMsgEvent> {
success: true,
};
} catch (error) {
this.logger.error('Failed to process commit-msg hook', { error });
const err = error instanceof Error ? error : new Error(String(error));
this.logger.error('Failed to process commit-msg hook', {
error: err.message,
stack: err.stack,
});
return {
handler: 'CommitMsgHandler',
success: false,
error: error instanceof Error ? error : new Error(String(error)),
error: err,
};
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/application/handlers/PostCommitHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,15 @@ export class PostCommitHandler implements IPostCommitHandler {
success: true,
};
} catch (error) {
this.logger?.error('Post-commit handler failed', { error });
const err = error instanceof Error ? error : new Error(String(error));
this.logger?.error('Post-commit handler failed', {
error: err.message,
stack: err.stack,
});
return {
handler: 'PostCommitHandler',
success: false,
error: error instanceof Error ? error : new Error(String(error)),
error: err,
};
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/application/handlers/PromptSubmitHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,15 @@ export class PromptSubmitHandler implements IPromptSubmitHandler {
output,
};
} catch (error) {
this.logger?.error('Prompt submit handler failed', { error });
const err = error instanceof Error ? error : new Error(String(error));
this.logger?.error('Prompt submit handler failed', {
error: err.message,
stack: err.stack,
});
return {
handler: 'PromptSubmitHandler',
success: false,
error: error instanceof Error ? error : new Error(String(error)),
error: err,
};
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/application/handlers/SessionStartHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,15 @@ export class SessionStartHandler implements ISessionStartHandler {
output,
};
} catch (error) {
this.logger?.error('Session start handler failed', { error });
const err = error instanceof Error ? error : new Error(String(error));
this.logger?.error('Session start handler failed', {
error: err.message,
stack: err.stack,
});
return {
handler: 'SessionStartHandler',
success: false,
error: error instanceof Error ? error : new Error(String(error)),
error: err,
};
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/application/handlers/SessionStopHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,15 @@ export class SessionStopHandler implements ISessionStopHandler {
output: result.summary,
};
} catch (error) {
this.logger?.error('Session stop handler failed', { error });
const err = error instanceof Error ? error : new Error(String(error));
this.logger?.error('Session stop handler failed', {
error: err.message,
stack: err.stack,
});
return {
handler: 'SessionStopHandler',
success: false,
error: error instanceof Error ? error : new Error(String(error)),
error: err,
};
}
}
Expand Down
17 changes: 16 additions & 1 deletion src/infrastructure/logging/Logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,21 @@ const LEVEL_COLORS: Record<LogLevel, string> = {

const RESET = '\x1b[0m';

/** JSON.stringify that handles circular references and Error objects. */
function safeStringify(obj: unknown): string {
const seen = new WeakSet();
return JSON.stringify(obj, (_key, value) => {
if (value instanceof Error) {
return { message: value.message, stack: value.stack };
}
if (typeof value === 'object' && value !== null) {
if (seen.has(value)) return '[Circular]';
seen.add(value);
}
return value;
});
}
Comment on lines +25 to +38
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new safeStringify function lacks test coverage. This is critical functionality that prevents silent logger failures, and should have tests covering:

  • Circular reference detection (e.g., const obj = {}; obj.self = obj;)
  • Error object serialization (e.g., new Error('test'))
  • Error objects with circular references in their properties
  • Nested objects with mixed circular references and Error objects
  • Null and undefined values
  • Primitives (strings, numbers, booleans)

Consider adding a test file at tests/unit/infrastructure/logging/safeStringify.test.ts or adding these tests to the existing Logger.test.ts file.

Copilot uses AI. Check for mistakes.

function formatTimestamp(): string {
const now = new Date();
const pad = (n: number, len = 2) => String(n).padStart(len, '0');
Expand Down Expand Up @@ -85,7 +100,7 @@ export class Logger implements ILogger {
if (!this.isLevelEnabled(level)) return;

const merged = { ...this.bindings, ...context };
const contextStr = Object.keys(merged).length > 0 ? ` ${JSON.stringify(merged)}` : '';
const contextStr = Object.keys(merged).length > 0 ? ` ${safeStringify(merged)}` : '';
const timestamp = formatTimestamp();
const levelUpper = level.toUpperCase().padEnd(5);

Expand Down