Skip to content

Commit 5a1a5ab

Browse files
authored
refactor: remove Logger SDK interface and delete logger.ts (#122)
Remove the Logger class (debug npm module wrapper) from the entire SDK interface. All ~75 remaining logger.*() calls in route handlers and browser classes migrated to Effect structured logging. Logger parameter removed from all 4 abstract handler signatures in types.ts, cascading through 28+ route and infrastructure files. - Delete src/logger.ts (151 lines) - Remove Logger from handler signatures (HTTPRoute, BrowserHTTPRoute, WebSocketRoute, BrowserWebsocketRoute) - Remove per-request Logger instantiation from router.ts and server.ts - Remove Logger from container/bootstrap service registration - Migrate browsers.cdp.ts (28 calls) and browsers.playwright.ts (8 calls) - Migrate 10 shared route handler files to Effect logging - Keep debug npm dependency (still used by config.ts, sdk-utils.ts, utils.ts) Completes Phase 8B — zero Logger references remain in production code.
1 parent 87dc25b commit 5a1a5ab

32 files changed

Lines changed: 137 additions & 361 deletions

src/browserless.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as fs from 'fs/promises';
22
import * as path from 'path';
33

44
import {
5-
Logger as BlessLogger,
65
BrowserHTTPRoute,
76
BrowserManager,
87
BrowserWebsocketRoute,
@@ -67,7 +66,6 @@ export class Browserless extends EventEmitter {
6766
protected fileSystem: FileSystem;
6867
protected hooks: Hooks;
6968
protected limiter: Limiter;
70-
protected Logger: typeof BlessLogger;
7169
protected metrics: Metrics;
7270
protected monitoring: Monitoring;
7371
protected router: Router;
@@ -90,15 +88,13 @@ export class Browserless extends EventEmitter {
9088
fileSystem,
9189
hooks,
9290
limiter,
93-
Logger: LoggerOverride,
9491
metrics,
9592
monitoring,
9693
router,
9794
token,
9895
webhooks,
9996
videoManager,
10097
}: {
101-
Logger?: Browserless['Logger'];
10298
browserManager?: Browserless['browserManager'];
10399
config?: Browserless['config'];
104100
fileSystem?: Browserless['fileSystem'];
@@ -112,7 +108,6 @@ export class Browserless extends EventEmitter {
112108
videoManager?: Browserless['videoManager'];
113109
} = {}) {
114110
super();
115-
this.Logger = LoggerOverride ?? BlessLogger;
116111
this.config = config || new Config();
117112
this.metrics = metrics || new Metrics();
118113
this.token = token || new Token(this.config);
@@ -135,7 +130,7 @@ export class Browserless extends EventEmitter {
135130
);
136131
this.router =
137132
router ||
138-
new Router(this.config, this.browserManager, this.limiter, this.Logger);
133+
new Router(this.config, this.browserManager, this.limiter);
139134
}
140135

141136
// Filter out routes that are not able to work on the arm64 architecture
@@ -508,7 +503,6 @@ export class Browserless extends EventEmitter {
508503
this.token,
509504
this.router,
510505
this.hooks,
511-
this.Logger,
512506
);
513507

514508
// Initialize server-scoped OTLP runtime — must happen before any sessions.
@@ -570,7 +564,6 @@ export class Browserless extends EventEmitter {
570564
fileSystem: c.resolve<FileSystem>(Services.FileSystem),
571565
hooks: c.resolve<Hooks>(Services.Hooks),
572566
limiter: c.resolve<Limiter>(Services.Limiter),
573-
Logger: c.resolve<typeof BlessLogger>(Services.Logger),
574567
metrics: c.resolve<Metrics>(Services.Metrics),
575568
monitoring: c.resolve<Monitoring>(Services.Monitoring),
576569
router: c.resolve<Router>(Services.Router),

src/browsers/browser-launcher.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
EdgePlaywright,
1515
FirefoxPlaywright,
1616
Hooks,
17-
Logger,
1817
NotFound,
1918
Request,
2019
WebKitPlaywright,
@@ -82,10 +81,9 @@ export class BrowserLauncher {
8281
async getBrowserForRequest(
8382
req: Request,
8483
router: BrowserHTTPRoute | BrowserWebsocketRoute,
85-
logger: Logger,
8684
): Promise<BrowserInstance> {
8785
return Effect.runPromise(
88-
this.getBrowserForRequestEffect(req, router, logger),
86+
this.getBrowserForRequestEffect(req, router),
8987
);
9088
}
9189

@@ -96,7 +94,6 @@ export class BrowserLauncher {
9694
private getBrowserForRequestEffect(
9795
req: Request,
9896
router: BrowserHTTPRoute | BrowserWebsocketRoute,
99-
logger: Logger,
10097
): Effect.Effect<BrowserInstance> {
10198
const launcher = this;
10299

@@ -198,7 +195,6 @@ export class BrowserLauncher {
198195
blockAds,
199196
config: launcher.config,
200197
enableReplay,
201-
logger,
202198
userDataDir,
203199
});
204200

@@ -246,12 +242,6 @@ export class BrowserLauncher {
246242
const sessionId = getFinalPathSegment(browser.wsEndpoint()!)!;
247243
session.id = sessionId;
248244

249-
// Update logger context
250-
logger.setSessionContext({
251-
trackingId,
252-
sessionId,
253-
});
254-
255245
// Register session
256246
launcher.registry.register(browser, session);
257247

src/browsers/browsers.cdp.ts

Lines changed: 36 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import {
22
BLESS_PAGE_IDENTIFIER,
33
BrowserLauncherOptions,
44
Config,
5-
Logger,
65
Request,
76
ServerError,
87
chromeExecutablePath,
@@ -28,6 +27,7 @@ import puppeteerStealth from 'puppeteer-extra';
2827

2928
import { CDPProxy, ReplayCapableBrowser, ReplayCompleteParams, TabReplayCompleteParams } from '../cdp-proxy.js';
3029
import { CloudflareSolver } from '../session/cloudflare-solver.js';
30+
import { runForkInServer } from '../otel-runtime.js';
3131
import type { CloudflareConfig } from '../shared/cloudflare-detection.js';
3232
puppeteerStealth.use(StealthPlugin());
3333

@@ -40,7 +40,6 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
4040
protected browser: Browser | null = null;
4141
protected browserWSEndpoint: string | null = null;
4242
protected port?: number;
43-
protected logger: Logger;
4443
protected proxy = httpProxy.createProxyServer();
4544
protected executablePath = playwright.chromium.executablePath();
4645
// Flag to track when WE are creating a page (vs external clients like pydoll)
@@ -61,12 +60,10 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
6160
config,
6261
enableReplay,
6362
userDataDir,
64-
logger,
6563
}: {
6664
blockAds: boolean;
6765
config: Config;
6866
enableReplay?: boolean;
69-
logger: Logger;
7067
userDataDir: ChromiumCDP['userDataDir'];
7168
}) {
7269
super();
@@ -75,9 +72,8 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
7572
this.config = config;
7673
this.blockAds = blockAds;
7774
this.enableReplay = enableReplay ?? false;
78-
this.logger = logger;
7975

80-
this.logger.info(`Starting new ${this.constructor.name} instance`);
76+
runForkInServer(Effect.logInfo(`Starting new ${this.constructor.name} instance`));
8177
}
8278

8379
protected cleanListeners() {
@@ -140,62 +136,62 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
140136
// Attaching to external targets causes CDP command conflicts and timeouts.
141137
if (!this.pendingInternalPage) {
142138
const targetId = this.getTargetId(target);
143-
this.logger.trace(`Skipping external target ${targetId} (created by external CDP client)`);
139+
runForkInServer(Effect.logDebug(`Skipping external target ${targetId} (created by external CDP client)`));
144140
return;
145141
}
146142

147143
const page = await target.page().catch((e) => {
148-
this.logger.error(`Error in ${this.constructor.name} new page ${e}`);
144+
runForkInServer(Effect.logError(`Error in ${this.constructor.name} new page ${e}`));
149145
return null;
150146
});
151147

152148
if (page) {
153-
this.logger.trace(`Setting up file:// protocol request rejection`);
149+
runForkInServer(Effect.logDebug(`Setting up file:// protocol request rejection`));
154150

155151
page.on('error', (err) => {
156-
this.logger.error(err);
152+
runForkInServer(Effect.logError(String(err)));
157153
});
158154

159155
page.on('pageerror', (err) => {
160-
this.logger.warn(err);
156+
runForkInServer(Effect.logWarning(String(err)));
161157
});
162158

163159
page.on('framenavigated', (frame) => {
164-
this.logger.trace(`Navigation to ${frame.url()}`);
160+
runForkInServer(Effect.logDebug(`Navigation to ${frame.url()}`));
165161
});
166162

167163
page.on('console', (message) => {
168-
this.logger.trace(`${message.type()}: ${message.text()}`);
164+
runForkInServer(Effect.logDebug(`${message.type()}: ${message.text()}`));
169165
});
170166

171167
page.on('requestfailed', (req) => {
172-
this.logger.warn(`"${req.failure()?.errorText}": ${req.url()}`);
168+
runForkInServer(Effect.logWarning(`"${req.failure()?.errorText}": ${req.url()}`));
173169
});
174170

175171
page.on('request', async (request) => {
176-
this.logger.trace(`${request.method()}: ${request.url()}`);
172+
runForkInServer(Effect.logDebug(`${request.method()}: ${request.url()}`));
177173
if (
178174
!this.config.getAllowFileProtocol() &&
179175
request.url().startsWith('file://')
180176
) {
181-
this.logger.error(
177+
runForkInServer(Effect.logError(
182178
`File protocol request found in request to ${this.constructor.name}, terminating`,
183-
);
179+
));
184180
page.close().catch(noop);
185181
this.close();
186182
}
187183
});
188184

189185
page.on('response', async (response) => {
190-
this.logger.trace(`${response.status()}: ${response.url()}`);
186+
runForkInServer(Effect.logDebug(`${response.status()}: ${response.url()}`));
191187

192188
if (
193189
!this.config.getAllowFileProtocol() &&
194190
response.url().startsWith('file://')
195191
) {
196-
this.logger.error(
192+
runForkInServer(Effect.logError(
197193
`File protocol request found in response to ${this.constructor.name}, terminating`,
198-
);
194+
));
199195
page.close().catch(noop);
200196
this.close();
201197
}
@@ -233,7 +229,7 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
233229
return Effect.fn('chromiumCdp.close')({ self: this }, function*() {
234230
if (!this.browser) return;
235231

236-
this.logger.info(
232+
yield* Effect.logInfo(
237233
`Closing ${this.constructor.name} process and all listeners`,
238234
);
239235
this.emit('close');
@@ -249,13 +245,13 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
249245
);
250246

251247
if (!closed && chromeProcess?.pid) {
252-
this.logger.warn(
248+
yield* Effect.logWarning(
253249
`browser.close() timed out after 5s — SIGKILL Chrome pid ${chromeProcess.pid}`,
254250
);
255251
try {
256252
chromeProcess.kill('SIGKILL');
257253
} catch (e) {
258-
this.logger.warn(`SIGKILL failed: ${e}`);
254+
yield* Effect.logWarning(`SIGKILL failed: ${e}`);
259255
}
260256
}
261257

@@ -282,7 +278,7 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
282278
}: BrowserLauncherOptions): Effect.Effect<Browser> {
283279
return Effect.fn('chromiumCdp.launch')({ self: this }, function*() {
284280
this.port = yield* Effect.promise(() => getPort());
285-
this.logger.info(`${this.constructor.name} got open port ${this.port}`);
281+
yield* Effect.logInfo(`${this.constructor.name} got open port ${this.port}`);
286282

287283
const extensionLaunchArgs = options.args?.find((a) =>
288284
a.startsWith('--load-extension'),
@@ -441,24 +437,23 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
441437
};
442438
fs.writeFileSync(prefsPath, JSON.stringify(prefs));
443439
} catch (err) {
444-
this.logger.warn(`Failed to write Chrome Preferences: ${err}`);
440+
yield* Effect.logWarning(`Failed to write Chrome Preferences: ${err}`);
445441
}
446442
}
447443

448444
const launchFn = stealth
449445
? puppeteerStealth.launch.bind(puppeteerStealth)
450446
: puppeteer.launch.bind(puppeteer);
451447

452-
this.logger.info(
453-
finalOptions,
448+
yield* Effect.logInfo(
454449
`Launching ${this.constructor.name} Handler`,
455450
);
456451
this.browser = (yield* Effect.promise(() => launchFn(finalOptions))) as Browser;
457452

458453
this.browser.on('targetcreated', this.onTargetCreated.bind(this));
459454
this.running = true;
460455
this.browserWSEndpoint = this.browser.wsEndpoint();
461-
this.logger.info(
456+
yield* Effect.logInfo(
462457
`${this.constructor.name} is running on ${this.browserWSEndpoint}`,
463458
);
464459

@@ -502,9 +497,9 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
502497
);
503498
}
504499

505-
this.logger.info(
500+
runForkInServer(Effect.logInfo(
506501
`Proxying ${req.parsed.href} to ${this.constructor.name}`,
507-
);
502+
));
508503

509504
const shouldMakePage = req.parsed.pathname.includes(
510505
BLESS_PAGE_IDENTIFIER,
@@ -531,9 +526,9 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
531526
},
532527
(error) => {
533528
const msg = error instanceof Error ? error.message : String(error);
534-
this.logger.warn(
529+
runForkInServer(Effect.logWarning(
535530
`Proxy disconnect for PAGE session to ${this.constructor.name}: ${msg}`,
536-
);
531+
));
537532
this.close();
538533
return reject(error);
539534
},
@@ -552,15 +547,15 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
552547
);
553548
}
554549

555-
this.logger.info(
550+
runForkInServer(Effect.logInfo(
556551
`Proxying ${req.parsed.href} to ${this.constructor.name} ${this.browserWSEndpoint}`,
557-
);
552+
));
558553

559554
return new Promise<void>((resolve, reject) => {
560555
const close = once(async () => {
561-
this.logger.debug(
556+
runForkInServer(Effect.logDebug(
562557
`proxyWebSocket close triggered: browser=${!!this.browser} cdpProxy=${!!this.cdpProxy}`,
563-
);
558+
));
564559
this.browser?.off('close', close);
565560
this.browser?.process()?.off('close', close);
566561
socket.off('close', close);
@@ -604,7 +599,7 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
604599
);
605600

606601
await this.cdpProxy.connect();
607-
this.logger.trace('CDPProxy connected successfully');
602+
runForkInServer(Effect.logDebug('CDPProxy connected successfully'));
608603

609604
// Wire tab count callback for tab limit enforcement
610605
if (this.getTabCountCallback) {
@@ -633,9 +628,9 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
633628
};
634629

635630
setup().catch((error) => {
636-
this.logger.error(
631+
runForkInServer(Effect.logError(
637632
`Error proxying session to ${this.constructor.name}: ${error}`,
638-
);
633+
));
639634
this.cdpProxy?.close(); // Triggers scope close → cleans up partial WS connections
640635
this.cdpProxy = null;
641636
this.close();
@@ -658,7 +653,7 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
658653
await this.cdpProxy.sendReplayComplete(metadata);
659654
return true;
660655
} else {
661-
this.logger.warn('Cannot send replay complete: no CDPProxy available');
656+
runForkInServer(Effect.logWarning('Cannot send replay complete: no CDPProxy available'));
662657
return false;
663658
}
664659
}
@@ -670,7 +665,7 @@ export class ChromiumCDP extends EventEmitter implements ReplayCapableBrowser {
670665
await this.cdpProxy.sendTabReplayComplete(metadata);
671666
return true;
672667
} else {
673-
this.logger.warn('Cannot send tab replay complete: no CDPProxy available');
668+
runForkInServer(Effect.logWarning('Cannot send tab replay complete: no CDPProxy available'));
674669
return false;
675670
}
676671
}

0 commit comments

Comments
 (0)