Skip to content

fix: surface an unbilled run, and clamp the google-docs page cap - #7025

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix-billing-observability
Aug 24, 2026
Merged

fix: surface an unbilled run, and clamp the google-docs page cap#7025
waleedlatif1 merged 1 commit into
stagingfrom
fix-billing-observability

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Two places where a failure is reported as something smaller than it is.

A run that is never billed logs as a notification problem

lib/logs/execution/logger.ts:1389

} catch (e) {
  // Safety net: if a step above threw BEFORE the single record call, ensure
  // the run is still billed. Reconciliation is idempotent...
  try {
    await this.recordExecutionUsage(...)
  } catch {}
  execLog.warn('Usage threshold notification check failed (non-fatal)', { error: e })
}

The comment documents the safety net, not the bare catch {} inside it.

The outer try covers a user lookup, recordExecutionUsage, and the threshold-email calls. With a degraded database the lookup throws before the record call, the safety-net re-record hits the same degraded database and is swallowed, and the run is never billed. The only line emitted says the notification check failed and was non-fatal — true of the outer failure, badly wrong about the inner one.

That is the shape that keeps a revenue-losing incident out of alerting: the log asserts the opposite of what happened.

Now logs at error with the execution and workflow ids, saying the run may be unbilled. The outer warn still covers the email path it was written for.

google-docs can ask Drive for a negative page

// google-slides.ts:301
/** Last-page precision: never ask Drive for more files than the cap still allows. */
const remaining = maxDocs > 0 ? Math.max(0, maxDocs - previouslyFetched) : 0

// google-docs.ts:307  — no clamp
const remaining = maxDocs > 0 ? maxDocs - previouslyFetched : 0

Both then run:

if (maxDocs > 0 && documents.length > remaining) documents = documents.slice(0, remaining)

A negative remaining makes that guard true for any non-empty page, and slice counts a negative end from the end — so it keeps the leading documents and drops the trailing ones, where the cap says to keep none. The wrong subset, silently, with slicedSome set.

Reachable when maxDocs is lowered while a sync cursor persists. google-drive guards the same case a different way (an early return when previouslyFetched >= maxFiles); google-docs had neither, and its google-slides twin carries the fix under a comment marking it deliberate — it just never propagated.

Scope

The logging change is observability only. The clamp changes behavior exactly in the case that is currently wrong.

Testing

1653 tests passing across lib/logs and connectors; bun run type-check clean.

Two places where a failure is reported as something smaller than it is.

**A run that is never billed logs as a notification problem.** The usage
safety net re-records billing when an earlier step threw before the single
record call, and its own failure went into a bare `catch {}`. With a degraded
database the user lookup throws first, the re-record hits the same database and
is swallowed, and the only line emitted reads "Usage threshold notification
check failed (non-fatal)" — which is true of the outer failure and badly wrong
about the inner one. It now logs at error with the execution and workflow ids,
and says the run may be unbilled. The outer warn still covers the email path it
was written for.

**google-docs can ask Drive for a negative page.** `remaining` was
`maxDocs - previouslyFetched` unclamped, where its google-slides twin carries
`Math.max(0, …)` under the comment "Last-page precision". Both then run
`if (documents.length > remaining) documents = documents.slice(0, remaining)`,
and a negative `remaining` makes that guard true for any non-empty page while
`slice` counts from the end — keeping the leading documents and dropping the
trailing ones, where the cap says to keep none. Reachable when `maxDocs` is
lowered while a sync cursor persists. google-drive guards the same case with an
early return; google-docs had neither.
@vercel

vercel Bot commented Aug 24, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 24, 2026 1:59am

Request Review

@cursor

cursor Bot commented Aug 24, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches execution usage recording observability (billing-adjacent) and Google Docs sync capping. Logging is observability-only; the clamp only changes the previously incorrect over-cap case.

Overview
Fixes two cases where failures were under-reported or produced the wrong result.

When the billing safety net in completeWorkflowExecution also fails, it now logs an error that the run may be unbilled (with execution and workflow ids) instead of a silent catch {} that only left a non-fatal notification warning.

Google Docs listing clamps remaining with Math.max(0, …) so a lowered maxDocs against a persisted cursor cannot go negative. Negative remaining made slice keep the wrong subset of a page.

Reviewed by Cursor Bugbot for commit 9c2a46a. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR fixes two underreported failure cases.

  • Clamps the Google Docs pagination remainder to zero when a persisted sync count exceeds a lowered document cap.
  • Emits an execution-scoped error when the final usage-recording safety net fails, while retaining the existing non-fatal notification warning.

Confidence Score: 5/5

The PR appears safe to merge, with both changes narrowly correcting the described pagination and observability behavior.

The connector clamp terminates capped pagination without exposing deletion reconciliation to a partial sliced listing, and the new billing diagnostic preserves structured identifiers without changing execution control flow.

Important Files Changed

Filename Overview
apps/sim/connectors/google-docs/google-docs.ts Clamps the remaining document allowance so an exhausted cap returns no documents instead of applying negative slice semantics.
apps/sim/lib/logs/execution/logger.ts Adds a throw-safe, execution-scoped error diagnostic when the fallback billing record attempt fails.

Reviews (1): Last reviewed commit: "fix: surface an unbilled run, and clamp ..." | Re-trigger Greptile

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9c2a46a. Configure here.

executionId,
workflowId: updatedLog.workflowId,
})
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Safety-net error log is unreachable dead code

Medium Severity

The new catch (recordError) block and its execLog.error message are unreachable for the described failure scenario. recordExecutionUsage wraps its entire body in an internal try/catch (line 1789) that swallows all errors and only logs via statsLog.error — it never throws to its caller. When the database is degraded, the safety-net call silently returns 0 and the catch (recordError) never fires. The only log emitted remains the misleading "non-fatal" warning at line 1412, so unbilled runs are not actually surfaced at error level as intended.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9c2a46a. Configure here.

@waleedlatif1
waleedlatif1 merged commit 7e0d868 into staging Aug 24, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix-billing-observability branch August 24, 2026 02:08
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