Skip to content

feat: persist logs locally#2087

Open
janburzinski wants to merge 3 commits into
mainfrom
emdash/save-logs-e0xw8
Open

feat: persist logs locally#2087
janburzinski wants to merge 3 commits into
mainfrom
emdash/save-logs-e0xw8

Conversation

@janburzinski
Copy link
Copy Markdown
Collaborator

summary

  • save logs locally
  • redact pii
  • add opt in checkbox to feedback modal
image

@janburzinski janburzinski changed the title feat: persist logs feat: persist logs locally May 18, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR adds structured local log persistence for the Electron main and renderer processes, a redaction pipeline that strips secrets and PII before writing or attaching logs, and an opt-in "Include diagnostic logs" checkbox in the feedback modal that fetches the last 500 KB of redacted logs and attaches them to the Discord webhook submission.

  • file-logger.ts: sequential write queue with log rotation (5 × 5 MB files), a chain of regex-based secret/PII redaction rules, a trusted-sender IPC handler for renderer logs, and a getDiagnosticLogAttachment export wired to a new RPC endpoint.
  • shared/logger.ts: new LogSink abstraction and serializeLogValue/stringifyLogValue helpers that handle Error, BigInt, Symbol, circular references, and functions before structured-clone or file I/O.
  • feedback-modal: opt-in checkbox triggers a lazy loadDiagnosticLog callback that is only executed inside the hook after setSubmitting(true), with Discord file-count and 8 MB payload guards added around the existing attachment flow.

Confidence Score: 5/5

Safe to merge. The log pipeline is well-isolated, write errors are swallowed without disrupting the caller, and the IPC handler validates both the sender origin and payload before accepting any renderer log entry.

All changed paths are additive (new file logger, new sink wiring, new UI checkbox). Error handling and state management in the feedback flow are correct across all branches. The two findings are minor: a byte-vs-character count inconsistency in the IPC payload guard, and a diagnostic attachment that doesn't fall back to the previous rotated file when the active log is small.

src/main/lib/file-logger.ts — isWithinPayloadLimit and getDiagnosticLogAttachment have the two minor gaps noted in comments.

Important Files Changed

Filename Overview
src/main/lib/file-logger.ts New file implementing sequential log rotation, redaction pipeline, and IPC handler for renderer logs. The byte-vs-character inconsistency in isWithinPayloadLimit and the single-file read in getDiagnosticLogAttachment are minor gaps worth tidying.
src/shared/logger.ts Adds LogSinkEntry/LogSink types, serializeLogValue/stringifyLogValue helpers, and wires an optional sink into createLogger. Circular-reference handling and BigInt/Symbol serialization look correct.
src/renderer/utils/logger.ts Renderer logger gains an IPC sink that pre-serializes entries before forwarding them to the main process.
src/renderer/lib/components/feedback-modal/use-feedback-submit.ts Adds optional loadDiagnosticLog callback to handleSubmit; enforces Discord file-count and payload-size limits before fetching. All error paths correctly reset the submitting flag.
src/renderer/lib/components/feedback-modal/feedback-modal.tsx Adds the opt-in checkbox UI; loadDiagnosticLog is lazily constructed and only called after setSubmitting(true) inside the hook, keeping state transitions consistent.
src/main/index.ts Calls initializeFileLogger, registerProcessErrorLogging, and registerRendererLogHandler at app startup before the window is created. Ordering is safe.
src/main/lib/file-logger.test.ts Comprehensive redaction tests covering secrets, PII, PEM blocks, DSNs, vendor tokens, and platform-specific home-directory patterns.
src/main/core/app/controller.ts Exposes getDiagnosticLogAttachment over RPC so the renderer can request the log attachment on demand.
src/main/lib/logger.ts Wires writeLogEntry as the file-logger sink for the main-process logger. Minimal and correct.

Sequence Diagram

sequenceDiagram
    participant R as Renderer Process
    participant IPC as IPC (emdash:renderer-log)
    participant FL as FileLogger (main)
    participant FS as emdash.log

    R->>IPC: eventSend(entry with serialized input)
    IPC->>FL: isTrustedRendererSender + isWithinPayloadLimit
    FL->>FL: parseRendererLog → writeLogEntry
    FL->>FL: redactDiagnosticLog(JSON payload)
    FL->>FL: "rotateIfNeeded (>5 MB → shift .1–.5)"
    FL->>FS: appendFile(redacted line)

    note over R,FS: Feedback modal opt-in path

    R->>R: User checks Include diagnostic logs
    R->>IPC: rpc.app.getDiagnosticLogAttachment()
    IPC->>FL: getDiagnosticLogAttachment()
    FL->>FS: readFile(emdash.log)
    FL->>FL: trimToLineBoundary(last 500 KB)
    FL->>FL: redactDiagnosticLog
    FL-->>R: filename, mimeType, content
    R->>R: new File([content], filename)
    R->>R: fetch Discord webhook (FormData with files)
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/main/lib/file-logger.ts:181-187
`isWithinPayloadLimit` measures the serialized payload in UTF-16 code units (`.length`), but `RENDERER_LOG_PAYLOAD_LIMIT` is named and intended as a byte ceiling. A renderer payload dense with non-BMP characters (emoji, CJK, etc.) can pass the check while still exceeding 64 KB when written to disk as UTF-8, letting the per-renderer DoS guard be bypassed even by the trusted renderer.

```suggestion
function isWithinPayloadLimit(payload: unknown): boolean {
  try {
    return Buffer.byteLength(JSON.stringify(payload), 'utf8') <= RENDERER_LOG_PAYLOAD_LIMIT;
  } catch {
    return false;
  }
}
```

### Issue 2 of 2
src/main/lib/file-logger.ts:80-101
**Diagnostic attachment misses logs spanning a rotation boundary**

`getDiagnosticLogAttachment` reads only the active `emdash.log` file. When the log was rotated shortly before the user submits feedback (e.g., heavy activity filled the 5 MB cap, rotation occurred, then the crash happened), `emdash.log` can contain only a handful of lines while the bulk of the relevant context sits in `emdash.log.1`. The most actionable crash reports — submitted right after a rotation — would arrive nearly empty. If the active file's content is smaller than `DIAGNOSTIC_LOG_BYTES`, appending the tail of the `.1` file up to the remaining byte budget would give meaningfully better coverage.

Reviews (2): Last reviewed commit: "fix: address diagnostic log review comme..." | Re-trigger Greptile

Comment thread src/main/lib/file-logger.ts
Comment thread src/main/lib/file-logger.ts Outdated
Comment thread src/main/lib/file-logger.ts
@janburzinski
Copy link
Copy Markdown
Collaborator Author

@greptileai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant