Skip to content

🧹 [Code Health] Use structured logger in Relay Server #30

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] Use structured logger in Relay Server #30

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
31 changes: 16 additions & 15 deletions src/relay/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { WebSocketServer } from 'ws';
import express from 'express';
import { readFileSync } from 'fs';
import { config } from 'dotenv';
import { relayLogger } from '../utils/logger.js';
import { AgentRegistry } from './agent-registry.js';
import { AgentCards } from './agent-cards.js';
import { JobQueue } from './job-queue.js';
Expand All @@ -24,7 +25,7 @@ import { wsConnections, connectedAgents } from '../utils/metrics.js';
// Load environment
config();

console.log('[Relay] Prometheus metrics enabled - default metrics collected');
relayLogger.info('Prometheus metrics enabled - default metrics collected');

const DEFAULT_CONFIG: Partial<RelayConfig> = {
port: parseInt(process.env.PORT || '3000'),
Expand Down Expand Up @@ -207,7 +208,7 @@ export class RelayServer {
return;
}

console.log(`[Relay] New WebSocket connection`);
relayLogger.info(`New WebSocket connection`);

// Update metrics
wsConnections.inc();
Expand All @@ -229,7 +230,7 @@ export class RelayServer {
this.handleMessage(socket as any, result.data);
}
} catch (err) {
console.error('[Relay] Invalid message:', err);
relayLogger.error('Invalid message format', { 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, Error objects do not have enumerable properties (like message and stack). When passed directly to JSON.stringify (which relayLogger uses under the hood), they serialize to an empty object {}. This means the actual error details and stack trace will be completely lost in your structured logs.

To preserve this critical diagnostic information, extract the error's message and stack trace explicitly.

Suggested change
relayLogger.error('Invalid message format', { error: err });
relayLogger.error('Invalid message format', {
error: err instanceof Error ? err.message : String(err),
stack: err instanceof Error ? err.stack : undefined,
});

socket.send(JSON.stringify({
error: 'Invalid message format',
}));
Expand All @@ -240,7 +241,7 @@ export class RelayServer {
const agent = this.registry.getBySocket(socket as any);
this.clearPendingForSocket(socket as any, 'Socket disconnected');
if (agent) {
console.log(`[Relay] Agent disconnected: ${agent.agentId}`);
relayLogger.info(`Agent disconnected: ${agent.agentId}`);
this.registry.unregister(agent.agentId);
}
// Update metrics
Expand All @@ -249,7 +250,7 @@ export class RelayServer {
});

socket.on('error', (err) => {
console.error('[Relay] Socket error:', err);
relayLogger.error('Socket 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, Error objects do not have enumerable properties (like message and stack). When passed directly to JSON.stringify (which relayLogger uses under the hood), they serialize to an empty object {}. This means the actual error details and stack trace will be completely lost in your structured logs.

To preserve this critical diagnostic information, extract the error's message and stack trace explicitly.

        relayLogger.error('Socket error', {
          error: err instanceof Error ? err.message : String(err),
          stack: err instanceof Error ? err.stack : undefined,
        });

});
});
}
Expand Down Expand Up @@ -294,7 +295,7 @@ export class RelayServer {
return;
}

console.log(`[Relay] Agent registered: ${sanitizedId} with capabilities: ${agentInfo.capabilities.join(', ')}`);
relayLogger.info(`Agent registered: ${sanitizedId} with capabilities: ${agentInfo.capabilities.join(', ')}`);

socket.send(JSON.stringify({
success: true,
Expand Down Expand Up @@ -415,7 +416,7 @@ export class RelayServer {
pending.resolve(msg);
}

console.log(`[Relay] ${msg.action} from ${msg.from.agent_id} routed to ${pending.requesterAgentId}`);
relayLogger.info(`${msg.action} from ${msg.from.agent_id} routed to ${pending.requesterAgentId}`);
}

private trackPendingRequest(msg: DAPMessage, requesterSocket: any, targetAgentId?: string): void {
Expand Down Expand Up @@ -667,15 +668,15 @@ export class RelayServer {
const stale = this.registry.getStaleAgents(this.config.heartbeatTimeoutMs);

for (const agent of stale) {
console.log(`[Relay] Removing stale agent: ${agent.agentId}`);
relayLogger.info(`Removing stale agent: ${agent.agentId}`);
agent.socket.close(4002, 'Heartbeat timeout');
this.registry.unregister(agent.agentId);
}

// Clean old jobs
const removedJobs = this.jobQueue.cleanStale(7 * 24 * 60 * 60 * 1000); // 7 days
if (removedJobs.length > 0) {
console.log(`[Relay] Cleaned ${removedJobs.length} old jobs`);
relayLogger.info(`Cleaned ${removedJobs.length} old jobs`);
}
}, 60000);
}
Expand All @@ -688,12 +689,12 @@ export class RelayServer {
this.httpServer!.listen(this.config.port, this.config.host, () => {
const addr = this.httpServer!.address() as any;
this._boundPort = addr?.port || this.config.port;
console.log(` HTTP: http://${this.config.host}:${this._boundPort}`);
relayLogger.info(`HTTP server listening on http://${this.config.host}:${this._boundPort}`);

// Start HTTPS server if TLS enabled
if (this.config.enableTls && this.httpsServer) {
this.httpsServer.listen(this.config.tlsPort, this.config.host, () => {
console.log(` HTTPS: https://${this.config.host}:${this.config.tlsPort}`);
relayLogger.info(`HTTPS server listening on https://${this.config.host}:${this.config.tlsPort}`);
finish();
});
} else {
Expand All @@ -702,7 +703,7 @@ export class RelayServer {
});

const finish = () => {
console.log(`
relayLogger.info(`

@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.

medium

Logging a multi-line ASCII art banner using a structured JSON logger (relayLogger.info) will result in a single log entry with escaped newline characters (\n), making it extremely difficult to read in log aggregation tools (e.g., Kibana, Datadog, or CloudWatch).

Since relayLogger is a structured logger, it is highly recommended to log a clean, single-line message and pass the server configuration details as structured metadata instead. For example:

relayLogger.info('DAP Relay Server started successfully', {
  version: '1.0.0',
  host: this.config.host,
  port: this._boundPort,
  wsUrl: `ws://${this.config.host}:${this._boundPort}/ws`,
  apiKeys: this.config.apiKeys.length > 0 ? this.config.apiKeys : 'open',
  heartbeatIntervalMs: this.config.heartbeatIntervalMs,
  tls: this.config.enableTls ? 'enabled' : 'disabled'
});

╔════════════════════════════════════════════════════════╗
║ DAP Relay Server v1.0.0 ║
╠════════════════════════════════════════════════════════╣
Expand All @@ -722,7 +723,7 @@ export class RelayServer {
}

async stop(): Promise<void> {
console.log('[Relay] Shutting down...');
relayLogger.info('Shutting down...');

if (this.cleanupInterval) {
clearInterval(this.cleanupInterval);
Expand Down Expand Up @@ -751,7 +752,7 @@ export class RelayServer {
server.close(() => {
closed++;
if (closed === servers.length) {
console.log('[Relay] Server stopped');
relayLogger.info('Server stopped');
resolve();
}
});
Expand Down Expand Up @@ -835,7 +836,7 @@ if (import.meta.url === `file://${process.argv[1]}`) {
process.exit(0);
});

relay.start().catch(console.error);
relay.start().catch((err: any) => relayLogger.error('Failed to start server', { 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, Error objects do not have enumerable properties (like message and stack). When passed directly to JSON.stringify (which relayLogger uses under the hood), they serialize to an empty object {}. This means the actual error details and stack trace will be completely lost in your structured logs.

To preserve this critical diagnostic information, extract the error's message and stack trace explicitly.

  relay.start().catch((err: any) =>
    relayLogger.error('Failed to start server', {
      error: err instanceof Error ? err.message : String(err),
      stack: err instanceof Error ? err.stack : undefined,
    })
  );

}

export default RelayServer;