Skip to content

fix: return client error for payload size limit violations#2560

Merged
alepane21 merged 6 commits intomainfrom
ale/eng-9034-controlplane-exceeding-publish-operations-limits-should
Feb 26, 2026
Merged

fix: return client error for payload size limit violations#2560
alepane21 merged 6 commits intomainfrom
ale/eng-9034-controlplane-exceeding-publish-operations-limits-should

Conversation

@alepane21
Copy link
Copy Markdown
Contributor

@alepane21 alepane21 commented Feb 26, 2026

Return a client error instead of a connect error.

Summary by CodeRabbit

  • Refactor
    • Improved error handling for persisted operations publishing to use consistent, structured error responses instead of exceptions for validation, authentication, and storage failures.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
  • I have read the Contributors Guide.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 26, 2026

Walkthrough

The PR converts exception-based error handling in the publishPersistedOperations function to structured response objects. Multiple error paths now return response envelopes with EnumStatusCode and details instead of throwing ConnectError exceptions.

Changes

Cohort / File(s) Summary
Error handling refactor
controlplane/src/core/bufservices/persisted-operation/publishPersistedOperations.ts, controlplane/test/persisted-operations.test.ts
Replaced exception-based error handling (ConnectError, ResourceExhausted) with structured response envelopes across multiple error paths including payload size, authentication, validation, and storage failures. Removed ConnectError and Code imports. Updated test to assert response codes and details instead of catching exceptions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • fix: encode and validate client name #2124: Directly overlaps with this PR by modifying publishPersistedOperations.ts to change error-handling paths from exceptions to structured response objects, including clientName validation adjustments.
  • feat: max number of persistent operations per request #2477: Related to the "payload too large" error handling logic in publishPersistedOperations.ts; this PR replaces ConnectError exception flow with structured ERR responses while the other PR enforces MAX_PERSISTED_OPERATIONS limit.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: converting payload size limit violations from exceptions to structured client error responses.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)

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

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.59%. Comparing base (59c3b86) to head (405edc9).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2560      +/-   ##
==========================================
- Coverage   63.59%   63.59%   -0.01%     
==========================================
  Files         298      298              
  Lines       41917    41920       +3     
  Branches     4439     4439              
==========================================
  Hits        26657    26657              
- Misses      15238    15241       +3     
  Partials       22       22              
Files with missing lines Coverage Δ
.../persisted-operation/publishPersistedOperations.ts 64.28% <100.00%> (+0.51%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alepane21 alepane21 changed the title fix: return as client error fix: return payload limits as client error Feb 26, 2026
@alepane21 alepane21 changed the title fix: return payload limits as client error fix: return client error for payload size limit violations Feb 26, 2026
Copy link
Copy Markdown
Contributor

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

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

LGTM

@alepane21 alepane21 merged commit 1393f43 into main Feb 26, 2026
9 checks passed
@alepane21 alepane21 deleted the ale/eng-9034-controlplane-exceeding-publish-operations-limits-should branch February 26, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants