Skip to content

Feat/logging fixes #69

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
thedevyashsaini merged 2 commits into from
Jun 9, 2026
Merged

Feat/logging fixes #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e4802dd
fix: logger reads SCRAWN_DEBUG and SCRAWN_LOG_FILE from env, graceful…
thedevyashsaini Jun 8, 2026
31d98eb
fix: parseURLToTarget uses protocol-appropriate port (443 for HTTPS, …
thedevyashsaini Jun 8, 2026
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: 7 additions & 1 deletion packages/scrawn/src/core/scrawn.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,13 @@ export class Scrawn<
private parseURLToTarget(baseURL: string): string {
if (baseURL.includes("://")) {
const url = new URL(baseURL);
return `${url.hostname}:${url.port || ScrawnConfig.grpc.defaultPort}`;
const protocolPort =
url.protocol === "https:"
? "443"
: url.protocol === "http:"
? "80"
: ScrawnConfig.grpc.defaultPort.toString();
Comment on lines +286 to +291

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

P1 Using port 80 as the fallback for http: URLs is wrong for gRPC. gRPC plaintext transport does not run on port 80 — the SDK's own default is 8069. Any caller who passes http://my-server (without an explicit port) will now attempt to connect to port 80 and fail silently. Only the https: case has a defensible standard port (443 for TLS gRPC); the http: case should keep falling back to ScrawnConfig.grpc.defaultPort.

Suggested change
const protocolPort =
url.protocol === "https:"
? "443"
: url.protocol === "http:"
? "80"
: ScrawnConfig.grpc.defaultPort.toString();
const protocolPort =
url.protocol === "https:"
? "443"
: ScrawnConfig.grpc.defaultPort.toString();

return `${url.hostname}:${url.port || protocolPort}`;
}

return baseURL.includes(":")
Expand Down
37 changes: 24 additions & 13 deletions packages/scrawn/src/utils/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,34 @@ import fs from "fs";
import path from "path";
import chalk from "chalk";
import pino from "pino";
import { ScrawnConfig } from "../config.js";

type LogLevel = "info" | "warn" | "error" | "debug";

// ensure log file directory exists
const logFilePath = path.resolve(process.cwd(), "scrawn.log");
fs.mkdirSync(path.dirname(logFilePath), { recursive: true });
const logLevel = process.env.SCRAWN_DEBUG ? "debug" : "info";

// create pino instance writing to file
const baseLogger = pino(
{
name: "scrawn",
level: ScrawnConfig.logging.enableDebug ? "debug" : "info",
timestamp: pino.stdTimeFunctions.isoTime,
},
pino.destination(logFilePath)
);
const defaultLogFile = "scrawn.log";
const logFileName = process.env.SCRAWN_LOG_FILE || defaultLogFile;
const logFilePath = path.resolve(process.cwd(), logFileName);

let baseLogger: pino.Logger;
try {
fs.mkdirSync(path.dirname(logFilePath), { recursive: true });
baseLogger = pino(
{
name: "scrawn",
level: logLevel,
timestamp: pino.stdTimeFunctions.isoTime,
},
pino.destination(logFilePath)
);
} catch (err) {
if (process.env.SCRAWN_LOG_FILE) {
console.error(
`[Scrawn] Cannot write to SCRAWN_LOG_FILE="${logFileName}" — file logging disabled`
);
}
baseLogger = pino({ level: "silent" });
}
Comment on lines +25 to +32

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

P2 When writing to the default scrawn.log fails (read-only filesystem, Lambda/edge environments, etc.), the error is swallowed silently — no message is printed, and all SDK logging is disabled. The warning path is guarded by if (process.env.SCRAWN_LOG_FILE), so it only fires for explicitly configured paths. Users debugging a silent SDK in production have no indication that logging was disabled.

Suggested change
} catch (err) {
if (process.env.SCRAWN_LOG_FILE) {
console.error(
`[Scrawn] Cannot write to SCRAWN_LOG_FILE="${logFileName}" — file logging disabled`
);
}
baseLogger = pino({ level: "silent" });
}
} catch (err) {
console.error(
`[Scrawn] Cannot write to log file "${logFileName}" — file logging disabled`
);
baseLogger = pino({ level: "silent" });
}


export class ScrawnLogger {
constructor(private context: string = "Scrawn") {}
Expand Down
Loading