Skip to content

Only push usage payloads when WebRTC connection was established#2359

Merged
grzegorz-roboflow merged 1 commit into
mainfrom
refactor-push-usage-payloads-on-error
May 21, 2026
Merged

Only push usage payloads when WebRTC connection was established#2359
grzegorz-roboflow merged 1 commit into
mainfrom
refactor-push-usage-payloads-on-error

Conversation

@rafel-roboflow
Copy link
Copy Markdown
Contributor

@rafel-roboflow rafel-roboflow commented May 21, 2026

What does this PR do?

Previously usage payloads were always flushed when the modal function completed, including the case where no WebRTC connection could be established at all. Move the push so it only fires on the success path and on the "connection established but no frames" error path, and skip it entirely when the connection was never established.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Testing

  • I have tested this change locally

Test details:
Running locally

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in hard-to-understand areas
  • My changes generate no new warnings or errors
  • I have updated the documentation accordingly (if applicable)

Additional Context

N/A


Note

Medium Risk
Adjusts usage-flush behavior in the WebRTC Modal worker, which can affect billing/telemetry accuracy. Change is small and localized but touches usage reporting on error paths.

Overview
Usage tracking in webrtc_worker/modal.py is adjusted so usage_collector.push_usage_payloads() is not called when a WebRTC connection was never established.

Payloads are now flushed only on the normal success path and on the "connection established but no frames processed" failure path, avoiding sending usage for sessions that never connected.

Reviewed by Cursor Bugbot for commit 3a22cb0. Bugbot is set up for automated code reviews on this repo. Configure here.

Previously usage payloads were always flushed when the modal function
completed, including the case where no WebRTC connection could be
established at all. Move the push so it only fires on the success path
and on the "connection established but no frames" error path, and skip
it entirely when the connection was never established.
@grzegorz-roboflow grzegorz-roboflow enabled auto-merge (squash) May 21, 2026 10:41
Copy link
Copy Markdown

@cursor cursor Bot left a comment

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 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 3a22cb0. Configure here.

),
)
usage_collector.push_usage_payloads()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Recorded usage still pushed by background threads

High Severity

record_usage() is still called unconditionally (even when the connection was never established), but only push_usage_payloads() is skipped. The UsageCollector singleton has a background _collector_thread that periodically calls _enqueue_usage_payload() every flush_interval seconds (default 10s), which will enqueue and eventually send the recorded data anyway. Additionally, if the Modal container is reused for a subsequent request, the stale recorded data accumulates in self._usage and gets pushed on the next successful push_usage_payloads() call. Moving record_usage() inside the conditional (or guarding it with connection_established) is needed to actually prevent usage from being reported when no connection was established.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3a22cb0. Configure here.

@grzegorz-roboflow grzegorz-roboflow merged commit ccd0332 into main May 21, 2026
49 checks passed
@grzegorz-roboflow grzegorz-roboflow deleted the refactor-push-usage-payloads-on-error branch May 21, 2026 10:44
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.

2 participants