Skip to content

adjust logging - #1073

Merged
elie222 merged 1 commit into
mainfrom
chore/log
Dec 7, 2025
Merged

adjust logging#1073
elie222 merged 1 commit into
mainfrom
chore/log

adjust logging

cef1f01
Select commit
Loading
Failed to load commit list.
MacroscopeApp / Review for correctness succeeded Dec 5, 2025 in 4m 9s

1 issue identified (15 code objects reviewed).

• Merge Base: 41cbb6a
• Head: cef1f01

Details

✅ File Path Comments Posted
✅ apps/web/app/api/user/stats/newsletters/route.ts 0
❌ apps/web/app/api/watch/controller.ts 1
✅ apps/web/utils/actions/ai-rule.ts 0
✅ apps/web/utils/actions/report.ts 0
✅ apps/web/utils/ai/assistant/chat.ts 0
✅ apps/web/utils/ai/assistant/process-user-request.ts 0
✅ apps/web/utils/ai/mcp/mcp-tools.ts 0
✅ apps/web/utils/ai/report/fetch.ts 0
✅ apps/web/utils/email/microsoft.ts 0
✅ apps/web/utils/gmail/signature-settings.ts 0
✅ apps/web/utils/outlook/subscription-manager.ts 0
✅ apps/web/utils/user/merge-premium.ts 0
✅ apps/web/utils/webhook/process-history-item.ts 0

Filtered Issues Details

apps/web/app/api/user/stats/newsletters/route.ts
  • line 203: limitClause is built via string interpolation without validating that options.limit is non‑negative. A negative value (e.g., ?limit=-1) passes Zod coercion and is truthy, yielding LIMIT -1, which can cause a runtime SQL error in PostgreSQL (LIMIT must not be negative). Validate and clamp to a non‑negative integer (and optionally cap to a sane maximum) before constructing the clause. [ Out of scope ]
apps/web/app/api/watch/controller.ts
  • line 26: On the Microsoft provider success path, the code returns the Date from createManagedOutlookSubscription but does not persist it to the database (e.g., prisma.emailAccount.update({ watchEmailsExpirationDate })). This creates contract/side‑effect asymmetry with the non‑Microsoft path, which explicitly updates watchEmailsExpirationDate. If createManagedOutlookSubscription does not itself persist the expiration (or subscription id), the database remains stale, making downstream logic that relies on DB state (e.g., cleanup/scheduling/unwatch flows) believe the account is not being watched. [ Out of scope ]
  • line 31: On the non‑Microsoft provider success path, only result.expirationDate is persisted. If provider.watchEmails() also returns a subscription identifier (commonly required to unwatch), it is silently dropped with no explicit rejection or comment. This risks making later unwatch operations impossible if they rely on a stored watchEmailsSubscriptionId, and it violates the requirement to avoid silent data loss at data‑conversion boundaries. [ Out of scope ]
apps/web/utils/actions/ai-rule.ts
  • line 264: When prompts differ textually but the AI diff yields no rule changes, the function returns early without updating emailAccount.rulesPrompt. This contradicts the documented flow step to update the user's prompt and causes the user's edited prompt text to be discarded. The problematic early return occurs after computing the diff when !diff.addedRules.length && !diff.editedRules.length && !diff.removedRules.length, skipping the DB update of rulesPrompt even though oldPromptFile !== rulesPrompt. [ Out of scope ]
  • line 397: The function updates emailAccount.rulesPrompt and reports counts based on intended operations rather than confirmed successes, leading to inconsistent state and misleading results when some operations fail but are caught. Specifically: (1) In the create loop, errors other than duplicates are caught and only logged, yet createdRules is still reported as addedRules?.length and the prompt is updated; (2) In the delete path, if deleteRule throws a non-NotFound error, it is caught and the loop continues, removeRulesCount is incremented, and the prompt is later updated. This can persist a prompt that implies changes that did not actually apply, and overstate createdRules/removedRules in the returned response. [ Low confidence ]
  • line 411: The logging change passes the raw error object to logger.error instead of a stringified/serialized representation. If the logger expects JSON-serializable data, logging an Error with circular references or large nested properties can cause serialization failures or drop useful context, potentially throwing during logging or truncating logs. Previously it logged error instanceof Error ? error.message : String(error), which guaranteed serializable output. [ Code style ]
apps/web/utils/actions/report.ts
  • line 176: Unbounded parallel requests when enriching labels can trigger Gmail API rate limits and intermittent failures. In fetchGmailLabels, each user label is processed with an individual gmail.users.labels.get call inside a Promise.all over the entire userLabels set. With many labels, this can create a burst of concurrent requests, causing 429s or quota errors. Although per-label failures are caught and zeroed, this can degrade accuracy (counts all zero) and produce noisy logs. Use bounded concurrency (e.g., a limiter) or batch/sequential processing to keep request rates within API limits. [ Low confidence ]
apps/web/utils/ai/assistant/process-user-request.ts
  • line 46: Possible crash when messages is empty: the code unconditionally accesses messages[messages.length - 1].role at line 46. If messages.length === 0, messages[messages.length - 1] is undefined and reading .role throws a runtime TypeError. The function signature allows any messages: { role: "assistant" | "user"; content: string }[] (including an empty array). Add a guard for empty arrays before indexing, or enforce non-empty via validation. [ Out of scope ]
apps/web/utils/ai/mcp/mcp-tools.ts
  • line 105: Silent data loss when multiple connections exist for the same integration: toolsByIntegration is keyed by integration.id, and each iteration calls toolsByIntegration.set(integration.id, ...). If there are multiple mcpConnection rows for the same integration.id, later entries will overwrite earlier ones, dropping tools from prior connections without warning. Consider merging tool sets per integration or guarding against duplicates. [ Out of scope ]
  • line 111: Passing a raw error object to the logger may cause serialization/logging to throw (e.g., if the logger JSON-serializes the meta and the error contains circular references). This exception would be thrown inside the inner catch block, potentially escaping and triggering the outer catch; combined with the current outer error path, this can lead to leaked clients and lost processing of other integrations. [ Low confidence ]
  • line 134: Possible resource leak: if an exception escapes the outer try/catch after one or more MCP clients have been created and pushed to clients, the outer catch returns a cleanup that is a no-op, so those clients will never be closed. This can happen if any code after client creation throws outside the inner per-integration try/catch (e.g., a logging failure or an unexpected error during merging). Ensure previously created clients are closed on the outer error path. [ Out of scope ]
apps/web/utils/ai/report/fetch.ts
  • line 121: Inner catch logs the raw error object via logger.warn("Failed to process draft:", { error });. If the logger attempts to serialize contextual objects (e.g., JSON.stringify), a non-serializable or circular error can cause logger.warn to throw. Because this call is inside the try block for the whole function, a thrown log here would escape the inner catch and abort the entire outer try, falling into the outer catch and potentially returning an empty list — losing any templates accumulated so far. Previously, the code stringified error safely. Consider logging a safe projection (e.g., error instanceof Error ? { message: error.message, stack: error.stack } : { error: String(error) }) to avoid logger-induced exceptions. [ Low confidence ]
  • line 128: Outer catch logs the raw error object via logger.warn("Failed to fetch email templates:", { error });. If the logger serializes the context and the error is non-serializable or circular, logger.warn may throw, causing the function to reject instead of returning [] as intended by the catch. Previously, the code logged a stringified/normalized error, avoiding this risk. Use a safe projection of the error to ensure logging cannot throw and the function reliably returns an empty array on failure. [ Low confidence ]
apps/web/utils/email/microsoft.ts
  • line 1427: Potential runtime TypeError when accessing message.headers.from without verifying message.headers exists. If any message lacks a headers object, message.headers.from will throw. Add a guard (e.g., check message.headers and message.headers.from or use optional chaining) before constructing the return value. [ Out of scope ]
apps/web/utils/user/merge-premium.ts
  • line 97: isOnHigherTier(targetTier, sourceTier) is called when both users have a premiumId (branch starting at if (sourceUser.premiumId && targetUser.premiumId)), but there is no explicit check that sourceUser.premium and targetUser.premium are non-null. If a premiumId exists but the related premium record is not loaded/resolvable, sourceTier or targetTier may be undefined, potentially causing isOnHigherTier to throw or mis-evaluate. Add explicit null checks (or defaulting) before invoking isOnHigherTier. [ Low confidence ]
  • line 170: In the premium admin transfer branch, two logically coupled updates are issued concurrently: prisma.premium.update({ data: { admins: { connect: { id: targetUserId }}}}) and conditionally prisma.user.update({ data: { premiumAdminId: sourceUser.premiumAdminId }}). Without a transaction and with parallel execution, failure of either can leave an inconsistent state (e.g., premiumAdminId points to an admin group the user is not actually connected to as admin). Execute these updates within a single prisma.$transaction([...]) to ensure consistency, and consider ordering or idempotency checks if needed. [ Low confidence ]
apps/web/utils/webhook/process-history-item.ts
  • line 49: markMessageAsProcessing({ userEmail, messageId }) sets a processing marker, but there is no corresponding clear/unmark on any success or early-return path, nor in a finally. If the marker is not TTL-based, any early return (e.g., ignored sender, not-inbox, assistant paths) or thrown error will leave the message permanently marked as processing, causing future attempts to skip processing (isFree === false). Add a finally that reliably clears the marker on all exit paths, or ensure the helper uses a short TTL. [ Low confidence ]
  • line 121: processAssistantEmail(...) is returned without await, so any rejection thrown by that promise will bypass the surrounding try/catch and skip the provider-specific not-found handling and error logging. This creates inconsistent error handling compared to other paths (e.g., handleOutboundMessage is awaited) and can lead to unhandled rejection at the call site. Replace return processAssistantEmail(...) with await processAssistantEmail(...) followed by return; to keep errors within the try/catch scope. [ Low confidence ]
  • line 175: Same as above: extractEmailAddress(parsedMessage.headers.from) result is used in prisma.newsletter.findUnique({ where: { email_emailAccountId: { email: sender, emailAccountId } } }) without verifying sender is a non-empty string. If sender is undefined, Prisma will throw at runtime. Add a guard before querying or skip categorization when the email cannot be extracted. [ Low confidence ]
View more details on MacroscopeApp