Skip to content

Improve logger#928

Merged
elie222 merged 2 commits intomainfrom
feat/logger-improvement
Nov 8, 2025
Merged

Improve logger#928
elie222 merged 2 commits intomainfrom
feat/logger-improvement

Conversation

@elie222
Copy link
Owner

@elie222 elie222 commented Nov 8, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved error tracking and logging with account context and additional diagnostics.
    • Added retry logic for Outlook API calls and clearer logging when calendar/Gmail/Outlook tokens are missing.
  • Improvements

    • AI generation flows now receive richer account context for more accurate, personalized results.
    • Enhanced error formatting and telemetry payloads; added user-aliasing support in analytics.
  • Other

    • Patch version bump.

@vercel
Copy link

vercel bot commented Nov 8, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
inbox-zero Ready Ready Preview Nov 8, 2025 4:43pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 8, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

This PR changes AI generation APIs to accept an emailAccount object (and thread emailAccount.id into error handling), adds emailAccountId to error/telemetry calls and webhook handlers, removes the server-action instrumentation middleware, and wraps several Outlook thread calls with a retry helper.

Changes

Cohort / File(s) Summary
LLM public API
apps/web/utils/llms/index.ts
createGenerateText and createGenerateObject now accept emailAccount (Pick<EmailAccountWithAI,"email"
AI call sites (mass update)
apps/web/utils/ai/... (many files, e.g. assistant/process-user-request.ts, calendar/availability.ts, categorize-sender/*, choose-rule/*, clean/*, digest/*, group/*, knowledge/*, mcp/*, reply/*, report/*, rule/*, snippets/*, cold-email/is-cold-email.ts)
~50+ call sites updated to pass emailAccount object instead of emailAccount.email/userEmail; no other control-flow changes.
Webhook & error telemetry
apps/web/utils/webhook/error-handler.ts, apps/web/utils/error.server.ts, apps/web/app/api/google/webhook/route.ts, apps/web/app/api/outlook/webhook/route.ts, apps/web/utils/middleware.ts
Added emailAccountId to webhook/error-handler options and logErrorToPosthog/trackError calls; Google/Outlook webhook handlers pass "unknown" placeholder in some error paths.
Clients: token error logging
apps/web/utils/gmail/client.ts, apps/web/utils/calendar/client.ts, apps/web/utils/outlook/client.ts
When refresh token is missing, code now logs an error including emailAccountId before throwing the same SafeError.
Outlook retry
apps/web/utils/outlook/thread.ts
Wrapped Outlook thread HTTP calls with withOutlookRetry(() => ...) for retry behavior; signatures unchanged.
PostHog & logger
apps/web/utils/posthog.ts, apps/web/utils/logger.ts
trackError now accepts/propagates emailAccountId; added aliasPosthogUser; improved error serialization and redaction list.
Middleware removal
apps/web/utils/actions/middleware.ts
Removed file and exported withActionInstrumentation wrapper plus its error-handling/instrumentation logic.
Version
version.txt
Bumped version v2.18.5 → v2.18.6.

Sequence Diagram(s)

sequenceDiagram
  participant Caller as App code (AI call site)
  participant LLM as llms.createGenerate{Text|Object}
  participant Model as LLM runtime
  participant Handler as llms.handleError
  participant PostHog as posthog.trackError

  Caller->>LLM: call with { emailAccount, label, modelOptions }
  LLM->>Model: generate
  alt success
    Model-->>LLM: result
    LLM-->>Caller: result
  else error
    Model-->>LLM: error
    LLM->>Handler: handleError(error, emailAccount.email, emailAccount.id, label, modelName)
    Handler->>PostHog: trackError({ email: emailAccount.email, emailAccountId: emailAccount.id, ... })
    Handler-->>Caller: throw / fallback
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • Consistency of ~50 AI call-site updates (ensure correct property usage: emailAccount.email vs emailAccount where required).
    • apps/web/utils/llms/index.ts signature and handleError changes (impact on all callers and telemetry).
    • Removal of withActionInstrumentation and any callers expecting that middleware.
    • Webhook/error paths that pass "unknown" as emailAccountId (placeholders and TODOs).
    • Outlook retry wrapper behavior to avoid double-retries.

Possibly related PRs

Suggested reviewers

  • anakarentorosserrano-star
  • johnlowe399-blip

Poem

🐰
I hopped through code both near and far,
Swapped emails for accounts — now that's the star,
IDs traced through errors, logs in line,
Webhooks, LLMs, PostHog all align,
A tidy hop — version bumped with a twirl. ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Improve logger' is vague and does not accurately reflect the scope of changes, which include refactoring AI utilities, webhook error handling, adding emailAccountId tracking, and removing middleware. The PR makes extensive changes across multiple systems, not just logger improvements. Use a more specific title like 'Add emailAccountId tracking to error handling and refactor AI utilities' or break into multiple PRs with focused scopes.
Docstring Coverage ⚠️ Warning Docstring coverage is 2.63% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/logger-improvement

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 33b5849 and fbece12.

📒 Files selected for processing (2)
  • apps/web/utils/logger.ts (2 hunks)
  • version.txt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/utils/logger.ts
🧰 Additional context used
📓 Path-based instructions (2)
!{.cursor/rules/*.mdc}

📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)

Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location

Files:

  • version.txt
!pages/_document.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.

Files:

  • version.txt
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-09-17T22:05:28.646Z
Learning: Applies to apps/web/utils/ai/**/*.{ts,tsx} : Use descriptive scoped loggers for each feature
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-07-18T17:27:46.389Z
Learning: Applies to **/api/**/route.ts : Use `withEmailAccount` middleware for API routes that operate on a specific email account (i.e., use or require `emailAccountId`).
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-07-18T15:04:30.467Z
Learning: Applies to apps/web/app/api/**/route.ts : Use `withEmailAccount` for email-account-level operations
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-07-18T17:27:58.249Z
Learning: Applies to apps/web/utils/actions/*.ts : Use `actionClient` when both authenticated user context and a specific `emailAccountId` are needed. The `emailAccountId` must be bound when calling the action from the client.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: test
🔇 Additional comments (1)
version.txt (1)

1-1: Version bump looks appropriate.

The patch version increment aligns with the logger improvements and related error-handling refinements in this PR.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/web/utils/middleware.ts (1)

105-105: Consider extracting emailAccountId from request when available.

The hardcoded "unknown" value is a placeholder (as indicated by the TODO comment). You could improve this by attempting to extract the emailAccountId from the request when it's available:

-        await logErrorToPosthog("api", req.url, apiError.type, "unknown"); // TODO: add emailAccountId
+        const emailAccountId = 
+          "auth" in req && "emailAccountId" in (req as RequestWithEmailAccount).auth
+            ? (req as RequestWithEmailAccount).auth.emailAccountId
+            : "unknown";
+        await logErrorToPosthog("api", req.url, apiError.type, emailAccountId);

This would provide better telemetry for errors that occur in requests with email account context, while still falling back to "unknown" for unauthenticated or non-account-specific requests.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 49 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="apps/web/utils/logger.ts">

<violation number="1" location="apps/web/utils/logger.ts:136">
Returning errorFull here forwards the unsanitized Error object to Axiom in production, undoing the sanitization this function was meant to enforce. Please avoid sending the raw error.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@elie222 elie222 merged commit 0e940c0 into main Nov 8, 2025
13 of 14 checks passed
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed changes from recent commits (found 1 issue).

1 issue found across 2 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="apps/web/utils/logger.ts">

<violation number="1" location="apps/web/utils/logger.ts:145">
Serializing the Error into a plain object drops non-enumerable fields like `.cause`, so nested error context is lost in production logs.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

function serializeError(error: unknown): unknown {
if (error instanceof Error) {
// Convert Error instance to plain object so hashSensitiveFields can process it
const serialized: Record<string, unknown> = {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Serializing the Error into a plain object drops non-enumerable fields like .cause, so nested error context is lost in production logs.

Prompt for AI agents
Address the following comment on apps/web/utils/logger.ts at line 145:

<comment>Serializing the Error into a plain object drops non-enumerable fields like `.cause`, so nested error context is lost in production logs.</comment>

<file context>
@@ -130,13 +130,37 @@ function formatError(args?: Record&lt;string, unknown&gt;) {
+function serializeError(error: unknown): unknown {
+  if (error instanceof Error) {
+    // Convert Error instance to plain object so hashSensitiveFields can process it
+    const serialized: Record&lt;string, unknown&gt; = {
+      name: error.name,
+      message: error.message,
</file context>
Fix with Cubic

@elie222 elie222 deleted the feat/logger-improvement branch December 18, 2025 23:03
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

Comments