Skip to content

Commit 3d32d03

Browse files
authored
refactor: eliminate internal Logger (debug npm) in favor of Effect structured logging (#115)
Remove all `new Logger(...)` instantiations used for internal logging across 24 files. Effect logging (logInfo, logWarning, logError, logDebug) flows through OTLP → Loki with structured attributes and span correlation, replacing the debug npm module's unstructured stderr output. Conversion patterns by context: - Effect generators: yield* Effect.logInfo(...) - Sync/callback: runForkInServer(Effect.logWarning(...)) - Own ManagedRuntime (encoder, cdp-session): this.runtime.runFork(Effect.log*(...)) - Signal handlers/child processes: console.error(JSON.stringify({...})) The Logger class itself remains — it's embedded in the SDK interface (types.ts handler signatures, browser constructors, per-request instantiation). That migration is Phase B (future work).
1 parent 0ecd8e6 commit 3d32d03

24 files changed

Lines changed: 326 additions & 237 deletions

src/browserless.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ type routeInstances =
6262
| BrowserWebsocketRoute;
6363

6464
export class Browserless extends EventEmitter {
65-
protected logger: BlessLogger;
6665
protected browserManager: BrowserManager;
6766
protected config: Config;
6867
protected fileSystem: FileSystem;
@@ -114,7 +113,6 @@ export class Browserless extends EventEmitter {
114113
} = {}) {
115114
super();
116115
this.Logger = LoggerOverride ?? BlessLogger;
117-
this.logger = new this.Logger('index');
118116
this.config = config || new Config();
119117
this.metrics = metrics || new Metrics();
120118
this.token = token || new Token(this.config);
@@ -158,8 +156,10 @@ export class Browserless extends EventEmitter {
158156
route.browser.name.toLowerCase().includes(b),
159157
)
160158
) {
161-
this.logger.warn(
162-
`Ignoring route "${route.path}" because it is not supported on arm64 platforms (route requires browser "${route.browser.name}").`,
159+
runForkInServer(
160+
Effect.logWarning(
161+
`Ignoring route "${route.path}" because it is not supported on arm64 platforms (route requires browser "${route.browser.name}").`,
162+
),
163163
);
164164
return false;
165165
}
@@ -486,8 +486,10 @@ export class Browserless extends EventEmitter {
486486
.filter((e, i, a) => a.findIndex((r) => r.name === e.name) !== i)
487487
.map((r) => r.name)
488488
.forEach((name) => {
489-
this.logger.warn(
490-
`Found duplicate routing names. Route names must be unique: ${name}`,
489+
runForkInServer(
490+
Effect.logWarning(
491+
`Found duplicate routing names. Route names must be unique: ${name}`,
492+
),
491493
);
492494
});
493495

src/browsers/browser-launcher.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { Page } from 'puppeteer-core';
3333
import micromatch from 'micromatch';
3434
import path from 'path';
3535

36+
import { runForkInServer } from '../otel-runtime.js';
3637
import { SessionRegistry } from '../session/session-registry.js';
3738
import { SessionCoordinator } from '../session/session-coordinator.js';
3839

@@ -57,8 +58,6 @@ export class BrowserLauncher {
5758
FirefoxPlaywright.name,
5859
WebKitPlaywright.name,
5960
];
60-
private log = new Logger('browser-launcher');
61-
6261
constructor(
6362
private config: Config,
6463
private hooks: Hooks,
@@ -268,14 +267,14 @@ export class BrowserLauncher {
268267
onTabReplayComplete: (metadata) => {
269268
if (isReplayCapable(browser)) {
270269
browser.sendTabReplayComplete(metadata).catch((e) => {
271-
launcher.log.warn(`Failed to send tab replay event: ${e instanceof Error ? e.message : String(e)}`);
270+
runForkInServer(Effect.logWarning(`Failed to send tab replay event: ${e instanceof Error ? e.message : String(e)}`));
272271
});
273272
}
274273
},
275274
onAntibotReport: (report) => {
276275
if (browser instanceof ChromiumCDP) {
277276
browser.emitAntibotReport(report).catch((e) => {
278-
launcher.log.warn(`Failed to emit antibot report: ${e instanceof Error ? e.message : String(e)}`);
277+
runForkInServer(Effect.logWarning(`Failed to emit antibot report: ${e instanceof Error ? e.message : String(e)}`));
279278
});
280279
}
281280
},
@@ -328,7 +327,7 @@ export class BrowserLauncher {
328327
if (found) {
329328
const [browser, session] = found;
330329
++session.numbConnected;
331-
this.log.debug(`Located browser with ID ${id}`);
330+
runForkInServer(Effect.logDebug(`Located browser with ID ${id}`));
332331
return browser;
333332
}
334333

@@ -370,7 +369,7 @@ export class BrowserLauncher {
370369
if (found) {
371370
const session = this.registry.get(found.browser)!;
372371
++session.numbConnected;
373-
this.log.debug(`Page connection: session ${session.id} numbConnected=${session.numbConnected} pageId=${id}`);
372+
runForkInServer(Effect.logDebug(`Page connection: session ${session.id} numbConnected=${session.numbConnected} pageId=${id}`));
374373
return found.browser;
375374
}
376375

src/browsers/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ import type { VideoManager } from '../video/video-manager.js';
4242
* This class was reduced from 1270 lines to ~200 lines.
4343
*/
4444
export class BrowserManager {
45-
protected log = new Logger('browser-manager');
4645
protected chromeBrowsers = [ChromiumCDP, ChromeCDP, EdgeCDP];
4746

4847
// Extracted components

src/cdp-proxy.ts

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ import WebSocket from 'ws';
33
const WebSocketServer = (WebSocket as any).Server as typeof import('ws').WebSocketServer;
44
import { Duplex } from 'stream';
55
import { IncomingMessage } from 'http';
6-
import { Config, Logger } from '@browserless.io/browserless';
6+
import { Config } from '@browserless.io/browserless';
77
import { Duration, Effect, Exit, FiberSet, Queue, Schedule, Schema, Scope, Stream } from 'effect';
88

99
import { incCounter, proxyDroppedMessages, wsLifecycle } from './effect-metrics.js';
10+
import { runForkInServer } from './otel-runtime.js';
1011
import { CloudflareConfig } from './shared/cloudflare-detection.js';
1112
import { CdpSessionId, TargetId } from './shared/cloudflare-detection.js';
1213
import { BROWSER_WS_PING_INTERVAL, BROWSER_WS_PONG_TIMEOUT_MS } from './session/cf/cf-schedules.js';
@@ -114,7 +115,6 @@ export class CDPProxy {
114115
private clientOutbound: Queue.Queue<ClientOutboundMessage> | null = null;
115116
private isClosing = false;
116117
private closeRequested = false;
117-
private log = new Logger('cdp-proxy');
118118
private getTabCount?: () => number;
119119
private readonly proxyScope = Scope.makeUnsafe();
120120
private readonly fibers = Effect.runSync(
@@ -190,7 +190,7 @@ export class CDPProxy {
190190
const ws = this.browserWs!;
191191
const onOpen = () => {
192192
ws.removeListener('error', onError);
193-
this.log.trace(`Connected to browser: ${this.browserWsEndpoint}`);
193+
runForkInServer(Effect.logDebug('Connected to browser').pipe(Effect.annotateLogs({ endpoint: this.browserWsEndpoint })));
194194
resume(Effect.void);
195195
};
196196
const onError = (err: Error) => {
@@ -222,7 +222,7 @@ export class CDPProxy {
222222
// Setup AFTER successful upgrade
223223
this.clientWs = clientWs;
224224
Effect.runSync(incCounter(wsLifecycle, { type: 'proxy_client', action: 'create' }));
225-
this.log.trace('Client WebSocket upgraded');
225+
runForkInServer(Effect.logDebug('Client WebSocket upgraded'));
226226

227227
// Scope-bound outbound queue: all client WS sends go through here.
228228
this.clientOutbound = Effect.runSync(Queue.unbounded<ClientOutboundMessage>());
@@ -254,12 +254,12 @@ export class CDPProxy {
254254
const sessionId = this.browserWsEndpoint.split('/').pop() || '';
255255
if (sessionId) {
256256
this.emitClientEvent('Browserless.sessionInfo', { sessionId }).catch((e) => {
257-
this.log.debug(`Failed to emit sessionInfo: ${e instanceof Error ? e.message : String(e)}`);
257+
runForkInServer(Effect.logDebug('Failed to emit sessionInfo').pipe(Effect.annotateLogs({ error: e instanceof Error ? e.message : String(e) })));
258258
});
259259
}
260260

261261
clientWs.on('error', (err: Error) => {
262-
this.log.warn(`Client WebSocket error: ${err.message}`);
262+
runForkInServer(Effect.logWarning('Client WebSocket error').pipe(Effect.annotateLogs({ error: err.message })));
263263
this.handleClose();
264264
});
265265
})(),
@@ -366,7 +366,7 @@ export class CDPProxy {
366366
// Sync check — fast path when CdpSession is tracking targets
367367
const count = this.getTabCount();
368368
if (count >= limit) {
369-
this.log.warn(`Tab limit reached (${count}/${limit}), rejecting Target.createTarget`);
369+
runForkInServer(Effect.logWarning('Tab limit reached, rejecting Target.createTarget').pipe(Effect.annotateLogs({ count: String(count), limit: String(limit) })));
370370
void this.sendClientError(msg.id, -32000, `Tab limit exceeded (${count}/${limit})`);
371371
return;
372372
}
@@ -390,9 +390,10 @@ export class CDPProxy {
390390
const clickCount = p?.clickCount ?? 0;
391391
// Full CDP sessionId — maps to a specific target (tab/OOPIF)
392392
const cdpSessionId = msg.sessionId ?? 'page';
393-
this.log.warn(
394-
`[PYDOLL-MOUSE] ${type} x=${x} y=${y} button=${button} clicks=${clickCount} cdpSession=${cdpSessionId}`,
395-
);
393+
runForkInServer(Effect.logWarning('[PYDOLL-MOUSE] dispatch').pipe(Effect.annotateLogs({
394+
type: String(type), x: String(x), y: String(y), button: String(button),
395+
clickCount: String(clickCount), cdpSession: String(cdpSessionId),
396+
})));
396397
}
397398
} catch {
398399
// ignore parse errors
@@ -407,7 +408,7 @@ export class CDPProxy {
407408
if (msg.method) {
408409
const sid = msg.sessionId ? ` [sid=${msg.sessionId.substring(0, 16)}]` : '';
409410
const params = msg.params ? JSON.stringify(msg.params).substring(0, 200) : '{}';
410-
this.log.info(`[CDP→Chrome] id=${msg.id} ${msg.method}${sid} ${params}`);
411+
runForkInServer(Effect.logInfo('[CDP→Chrome]').pipe(Effect.annotateLogs({ id: String(msg.id), method: msg.method, sid: sid.trim(), params })));
411412
}
412413
} catch { /* ignore */ }
413414
}
@@ -431,7 +432,7 @@ export class CDPProxy {
431432
if (this.cdpDebug && msg.method) {
432433
const sid = msg.sessionId ? ` [sid=${msg.sessionId.substring(0, 16)}]` : '';
433434
const params = msg.params ? JSON.stringify(msg.params).substring(0, 150) : '{}';
434-
this.log.info(`[Chrome→CDP] ${msg.method}${sid} ${params}`);
435+
runForkInServer(Effect.logInfo('[Chrome→CDP]').pipe(Effect.annotateLogs({ method: msg.method, sid: sid.trim(), params })));
435436
}
436437
}
437438
} catch { /* ignore parse errors */ }
@@ -443,12 +444,12 @@ export class CDPProxy {
443444

444445
// Handle close from either side
445446
this.clientWs.on('close', () => {
446-
this.log.trace('Client WebSocket closed');
447+
runForkInServer(Effect.logDebug('Client WebSocket closed'));
447448
this.handleClose();
448449
});
449450

450451
this.browserWs.on('close', () => {
451-
this.log.trace('Browser WebSocket closed');
452+
runForkInServer(Effect.logDebug('Browser WebSocket closed'));
452453
this.handleClose();
453454
});
454455

@@ -469,7 +470,7 @@ export class CDPProxy {
469470
Effect.tryPromise(() => this.onBeforeClose!()).pipe(
470471
Effect.timeout(Duration.millis(ON_BEFORE_CLOSE_TIMEOUT_MS)),
471472
Effect.catch((e) => Effect.sync(() => {
472-
this.log.warn(`onBeforeClose failed: ${e instanceof Error ? e.message : String(e)}`);
473+
runForkInServer(Effect.logWarning('onBeforeClose failed').pipe(Effect.annotateLogs({ error: e instanceof Error ? e.message : String(e) })));
473474
})),
474475
),
475476
);
@@ -490,7 +491,7 @@ export class CDPProxy {
490491
if (!Queue.offerUnsafe(this.clientOutbound!, {
491492
data: message,
492493
onSent: resolve,
493-
onError: (err) => { this.log.warn(`Failed to send CDP response id=${id}: ${err.message}`); reject(err); },
494+
onError: (err) => { runForkInServer(Effect.logWarning('Failed to send CDP response').pipe(Effect.annotateLogs({ id: String(id), error: err.message }))); reject(err); },
494495
})) {
495496
Effect.runSync(incCounter(proxyDroppedMessages, { direction: 'client' }));
496497
resolve();
@@ -505,7 +506,7 @@ export class CDPProxy {
505506
if (!Queue.offerUnsafe(this.clientOutbound!, {
506507
data: payload,
507508
onSent: resolve,
508-
onError: (err) => { this.log.warn(`Failed to send CDP error id=${id}: ${err.message}`); reject(err); },
509+
onError: (err) => { runForkInServer(Effect.logWarning('Failed to send CDP error').pipe(Effect.annotateLogs({ id: String(id), error: err.message }))); reject(err); },
509510
})) {
510511
Effect.runSync(incCounter(proxyDroppedMessages, { direction: 'client' }));
511512
resolve();
@@ -528,13 +529,13 @@ export class CDPProxy {
528529
const targets: Array<{ type: string }> = result?.targetInfos ?? [];
529530
const count = targets.filter(t => t.type === 'page').length;
530531
if (count >= limit) {
531-
this.log.warn(`Tab limit reached (${count}/${limit}), rejecting Target.createTarget`);
532+
runForkInServer(Effect.logWarning('Tab limit reached, rejecting Target.createTarget').pipe(Effect.annotateLogs({ count: String(count), limit: String(limit) })));
532533
void this.sendClientError(msgId, -32000, `Tab limit exceeded (${count}/${limit})`);
533534
return;
534535
}
535536
} catch (e) {
536537
// If we can't determine tab count, allow the request through
537-
this.log.debug(`Tab count check failed, allowing Target.createTarget: ${e instanceof Error ? e.message : String(e)}`);
538+
runForkInServer(Effect.logDebug('Tab count check failed, allowing Target.createTarget').pipe(Effect.annotateLogs({ error: e instanceof Error ? e.message : String(e) })));
538539
}
539540
// Under limit or check failed — forward to browser
540541
this.sendToBrowser(data, isBinary);
@@ -549,15 +550,15 @@ export class CDPProxy {
549550
*/
550551
async emitClientEvent(method: string, params: object): Promise<void> {
551552
if (!this.clientOutbound) {
552-
this.log.warn(`Cannot inject event ${method}: queue not initialized`);
553+
runForkInServer(Effect.logWarning('Cannot inject event: queue not initialized').pipe(Effect.annotateLogs({ method })));
553554
return;
554555
}
555556
const message = JSON.stringify({ method, params });
556557
return new Promise<void>((resolve, reject) => {
557558
const offered = Queue.offerUnsafe(this.clientOutbound!, {
558559
data: message,
559-
onSent: () => { this.log.trace(`Injected CDP event: ${method}`); resolve(); },
560-
onError: (err) => { this.log.warn(`Failed to inject CDP event ${method}: ${err.message}`); reject(err); },
560+
onSent: () => { runForkInServer(Effect.logDebug('Injected CDP event').pipe(Effect.annotateLogs({ method }))); resolve(); },
561+
onError: (err) => { runForkInServer(Effect.logWarning('Failed to inject CDP event').pipe(Effect.annotateLogs({ method, error: err.message }))); reject(err); },
561562
});
562563
if (!offered) {
563564
Effect.runSync(incCounter(proxyDroppedMessages, { direction: 'client' }));
@@ -590,7 +591,7 @@ export class CDPProxy {
590591
if (this.cdpDebug) {
591592
const sid = sessionId ? ` [sid=${sessionId.substring(0, 16)}]` : '';
592593
const p = JSON.stringify(params).substring(0, 200);
593-
this.log.info(`[SOLVER→Chrome] ${method}${sid} ${p}`);
594+
runForkInServer(Effect.logInfo('[SOLVER→Chrome]').pipe(Effect.annotateLogs({ method, sid: sid.trim(), params: p })));
594595
}
595596

596597
return conn.sendPromise(method, params, sessionId, timeoutMs);
@@ -678,12 +679,12 @@ export class CDPProxy {
678679
*/
679680
async sendReplayComplete(metadata: ReplayCompleteParams): Promise<void> {
680681
await this.emitClientEvent('Browserless.replayComplete', metadata);
681-
this.log.info(`Sent replay complete event: ${metadata.id}`);
682+
runForkInServer(Effect.logInfo('Sent replay complete event').pipe(Effect.annotateLogs({ replay_id: metadata.id })));
682683
}
683684

684685
async sendTabReplayComplete(metadata: TabReplayCompleteParams): Promise<void> {
685686
await this.emitClientEvent('Browserless.tabReplayComplete', metadata);
686-
this.log.info(`Sent tab replay complete event: targetId=${metadata.targetId}`);
687+
runForkInServer(Effect.logInfo('Sent tab replay complete event').pipe(Effect.annotateLogs({ targetId: metadata.targetId })));
687688
}
688689

689690
/**
@@ -732,15 +733,15 @@ export class CDPProxy {
732733
});
733734

734735
if (!gotPong) {
735-
this.log.warn('Browser WS heartbeat timeout — Chrome not responding, closing session');
736+
runForkInServer(Effect.logWarning('Browser WS heartbeat timeout — Chrome not responding, closing session'));
736737
this.handleClose();
737738
}
738739
});
739740

740741
this.forkManaged(
741742
tick().pipe(
742743
Effect.catch((e) => Effect.sync(() => {
743-
this.log.warn(`Browser WS ping failed, closing session: ${e instanceof Error ? e.message : String(e)}`);
744+
runForkInServer(Effect.logWarning('Browser WS ping failed, closing session').pipe(Effect.annotateLogs({ error: e instanceof Error ? e.message : String(e) })));
744745
this.handleClose();
745746
})),
746747
Effect.repeat(Schedule.fixed(BROWSER_WS_PING_INTERVAL)),
@@ -779,10 +780,10 @@ export class CDPProxy {
779780

780781
const clientState = this.clientWs?.readyState;
781782
const browserState = this.browserWs?.readyState;
782-
this.log.info(
783-
`CDPProxy closing: clientWs=${clientState === WebSocket.OPEN ? 'OPEN' : clientState} ` +
784-
`browserWs=${browserState === WebSocket.OPEN ? 'OPEN' : browserState}`
785-
);
783+
runForkInServer(Effect.logInfo('CDPProxy closing').pipe(Effect.annotateLogs({
784+
clientWs: clientState === WebSocket.OPEN ? 'OPEN' : String(clientState),
785+
browserWs: browserState === WebSocket.OPEN ? 'OPEN' : String(browserState),
786+
})));
786787

787788
// Await scope close so all acquireRelease finalizers fire before onClose
788789
this.closePromise = Effect.runPromise(Scope.close(this.proxyScope, Exit.void))

src/container/container.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import { Logger } from '@browserless.io/browserless';
1+
import { Effect } from 'effect';
2+
3+
import { runForkInServer } from '../otel-runtime.js';
24

35
/**
46
* Service factory function type.
@@ -36,7 +38,6 @@ interface ServiceRegistration<T> {
3638
export class ServiceContainer {
3739
private services: Map<string, ServiceRegistration<unknown>> = new Map();
3840
private resolving: Set<string> = new Set(); // For circular dependency detection
39-
private log = new Logger('container');
4041

4142
/**
4243
* Register a singleton service.
@@ -48,7 +49,7 @@ export class ServiceContainer {
4849
dependencies: string[] = []
4950
): this {
5051
if (this.services.has(name)) {
51-
this.log.warn(`Service "${name}" is being overwritten`);
52+
runForkInServer(Effect.logWarning(`Service "${name}" is being overwritten`));
5253
}
5354

5455
this.services.set(name, {
@@ -70,7 +71,7 @@ export class ServiceContainer {
7071
dependencies: string[] = []
7172
): this {
7273
if (this.services.has(name)) {
73-
this.log.warn(`Service "${name}" is being overwritten`);
74+
runForkInServer(Effect.logWarning(`Service "${name}" is being overwritten`));
7475
}
7576

7677
this.services.set(name, {
@@ -88,7 +89,7 @@ export class ServiceContainer {
8889
*/
8990
registerInstance<T>(name: string, instance: T): this {
9091
if (this.services.has(name)) {
91-
this.log.warn(`Service "${name}" is being overwritten with instance`);
92+
runForkInServer(Effect.logWarning(`Service "${name}" is being overwritten with instance`));
9293
}
9394

9495
this.services.set(name, {
@@ -160,7 +161,7 @@ export class ServiceContainer {
160161
* Call this at startup to fail fast.
161162
*/
162163
validate(): void {
163-
this.log.debug('Validating service container...');
164+
runForkInServer(Effect.logDebug('Validating service container...'));
164165

165166
// Check for missing dependencies
166167
for (const [name, registration] of this.services) {
@@ -200,7 +201,7 @@ export class ServiceContainer {
200201
visit(name);
201202
}
202203

203-
this.log.debug(`Validated ${this.services.size} services`);
204+
runForkInServer(Effect.logDebug(`Validated ${this.services.size} services`));
204205
}
205206

206207
/**

0 commit comments

Comments
 (0)