-
Notifications
You must be signed in to change notification settings - Fork 14
chore(code): add chromium logs #1974
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,13 @@ | ||||||||||
| import { existsSync, mkdirSync, renameSync, unlinkSync } from "node:fs"; | ||||||||||
| import { | ||||||||||
| closeSync, | ||||||||||
| existsSync, | ||||||||||
| fstatSync, | ||||||||||
| mkdirSync, | ||||||||||
| openSync, | ||||||||||
| readSync, | ||||||||||
| renameSync, | ||||||||||
| unlinkSync, | ||||||||||
| } from "node:fs"; | ||||||||||
| import os from "node:os"; | ||||||||||
| import { join } from "node:path"; | ||||||||||
| import { initOtelTransport } from "@main/utils/otel-log-transport"; | ||||||||||
|
|
@@ -57,3 +66,41 @@ export { shutdownOtelTransport } from "@main/utils/otel-log-transport"; | |||||||||
| export function getLogFilePath(): string { | ||||||||||
| return join(LOG_DIR, LOG_FILE); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| export function getChromiumLogFilePath(): string | undefined { | ||||||||||
| return process.env.POSTHOG_CODE_CHROMIUM_LOG_PATH; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const CHROMIUM_LOG_TAIL_BYTES = 32 * 1024; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Read the last ~32 KB of the Chromium internal log file. Used by crash | ||||||||||
| * handlers to attach the tail to OTEL/electron-log so PostHog gets the native | ||||||||||
| * V8/GPU output around a renderer death — Chromium writes chromium.log | ||||||||||
| * directly from native code and never goes through electron-log otherwise. | ||||||||||
| */ | ||||||||||
| export function readChromiumLogTail(): string | undefined { | ||||||||||
| const path = getChromiumLogFilePath(); | ||||||||||
| if (!path) return undefined; | ||||||||||
|
|
||||||||||
| let fd: number | undefined; | ||||||||||
| try { | ||||||||||
| fd = openSync(path, "r"); | ||||||||||
| const { size } = fstatSync(fd); | ||||||||||
| if (size === 0) return undefined; | ||||||||||
| const length = Math.min(size, CHROMIUM_LOG_TAIL_BYTES); | ||||||||||
| const buf = Buffer.alloc(length); | ||||||||||
| readSync(fd, buf, 0, length, size - length); | ||||||||||
| return buf.toString("utf8"); | ||||||||||
|
Comment on lines
+93
to
+94
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/code/src/main/utils/logger.ts
Line: 93-94
Comment:
**`readSync` return value not used to bound `toString`**
`readSync` returns the number of bytes actually read, which may be less than `length` (e.g. if the file was truncated between the `fstatSync` and `readSync` calls). Since `Buffer.alloc` zero-fills, any unread bytes at the end become null bytes in the UTF-8 string. Using `bytesRead` to slice the buffer is safer:
```suggestion
const bytesRead = readSync(fd, buf, 0, length, size - length);
return buf.subarray(0, bytesRead).toString("utf8");
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||
| } catch { | ||||||||||
| return undefined; | ||||||||||
| } finally { | ||||||||||
| if (fd !== undefined) { | ||||||||||
| try { | ||||||||||
| closeSync(fd); | ||||||||||
| } catch { | ||||||||||
| // Best-effort close | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
Comment on lines
+82
to
+106
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/code/src/main/utils/logger.ts
Line: 82-106
Comment:
**Chromium log file grows unboundedly**
`chromium.log` has no size limit or rotation, unlike `main.log` which is capped at 10 MB with 3 archives. With `--log-level=0` (VERBOSE), Chromium is extremely chatty during normal operation and can produce several megabytes per session. Over time this will silently consume disk space with no cleanup mechanism. The same log directory already implements rotation for the main log — the same approach (or at least a `maxSize` guard that truncates the file at startup) should be applied here.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same expression —
join(os.homedir(), ".posthog-code", isDev ? "logs-dev" : "logs")— already appears verbatim inlogger.tsasLOG_DIR. If the log directory ever changes, both sites need updating in sync. Consider extracting a shared helper (e.g. a smallutils/paths.tsexportinggetLogDir(isDev)) that bothbootstrap.tsandlogger.tscan import without creating a circular dependency.Prompt To Fix With AI