Skip to content

🧹 [Code Health] Replace console.log with structured Logger in dap-client.ts #32

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

Open
anusornc wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🧹 [Code Health] Replace console.log with structured Logger in dap-client.ts #32

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
17 changes: 10 additions & 7 deletions src/client/dap-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
AgentInfo,
Job,
} from '../protocol/types.js';
import { Logger } from '../utils/logger.js';

export interface DAPClientConfig {
relayUrl: string;
Expand All @@ -24,6 +25,8 @@ export interface DAPClientConfig {

export type MessageHandler = (msg: DAPMessage) => Promise<void>;

const clientLogger = new Logger('dap-client');

export class DAPClient {
private config: DAPClientConfig;
private socket: WebSocket | null = null;
Expand Down Expand Up @@ -57,7 +60,7 @@ export class DAPClient {
this.socket = new WebSocket(url.toString());

this.socket.onopen = () => {
console.log(`[DAPClient] Connected to ${this.config.relayUrl}`);
clientLogger.info(`Connected to ${this.config.relayUrl}`);

const registerMsg = {
action: 'register',
Expand All @@ -79,18 +82,18 @@ export class DAPClient {
const msg = JSON.parse(event.data) as DAPMessage;
await this.handleMessage(msg);
} catch (err) {
console.error('[DAPClient] Message handling error:', err);
clientLogger.error('Message handling error', { error: err });

@gemini-code-assist gemini-code-assist Bot Jun 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

In JavaScript/TypeScript, standard Error properties (like message and stack) are non-enumerable. When you pass an Error object directly inside the metadata object to JSON.stringify (which is what the structured Logger does internally), it serializes to {}. This means the actual error message and stack trace will be completely lost in your structured logs.

To fix this, convert the error to a plain object or extract its properties before logging.

          clientLogger.error('Message handling error', {
            error: err instanceof Error ? { message: err.message, stack: err.stack } : err,
          });

}
};

this.socket.onclose = (event) => {
console.log(`[DAPClient] Disconnected: ${event.code}`);
clientLogger.info(`Disconnected: ${event.code}`);
this.connected = false;
this.scheduleReconnect();
};

this.socket.onerror = (err) => {
console.error('[DAPClient] Connection error:', err);
clientLogger.error('Connection error', { error: err });

@gemini-code-assist gemini-code-assist Bot Jun 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Similar to the other error logging instances, passing the err object directly inside the metadata object will result in an empty object {} in the structured JSON logs if err is an Error instance. Please serialize the error properties explicitly to preserve the error details.

        clientLogger.error('Connection error', {
          error: err instanceof Error ? { message: err.message, stack: err.stack } : err,
        });

reject(err);
};
});
Expand Down Expand Up @@ -122,13 +125,13 @@ export class DAPClient {
60000
);

console.log(`[DAPClient] Reconnecting in ${delay}ms (attempt ${this.reconnectAttempts})`);
clientLogger.info(`Reconnecting in ${delay}ms (attempt ${this.reconnectAttempts})`);

this.reconnectTimer = setTimeout(async () => {
this.reconnectTimer = undefined;
try {
await this.connect();
console.log('[DAPClient] Reconnected');
clientLogger.info('Reconnected');
} catch {
// Will try again
}
Expand All @@ -152,7 +155,7 @@ export class DAPClient {
try {
await handler(msg);
} catch (err) {
console.error('[DAPClient] Handler error:', err);
clientLogger.error('Handler error', { error: err });

@gemini-code-assist gemini-code-assist Bot Jun 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Passing the caught err directly inside the metadata object will cause it to serialize as {} in the structured JSON logs because Error properties are non-enumerable. Please serialize the error properties explicitly to ensure the handler error details are captured.

        clientLogger.error('Handler error', {
          error: err instanceof Error ? { message: err.message, stack: err.stack } : err,
        });

}
}
}
Expand Down
Loading