Skip to content

Permissively parse SDPs unless completely invalid#3297

Merged
philipch07 merged 1 commit intomasterfrom
pch07/permissive-sdps-except-all-invalid
Dec 11, 2025
Merged

Permissively parse SDPs unless completely invalid#3297
philipch07 merged 1 commit intomasterfrom
pch07/permissive-sdps-except-all-invalid

Conversation

@philipch07
Copy link
Contributor

Description

Instead of throwing errors since there are numerous SDP types, we can be more permissive by allowing them to be ignored (with a logged warning) and instead bubble up the last error if they all failed.

This is a re-implementation of pion/sdp#233, but in a more appropriate place (see pion/sdp#233 (comment)).

Reference issue

Resolves #1315.

@philipch07 philipch07 requested a review from JoTurk December 11, 2025 05:15
@JoTurk JoTurk added this to the V4.2.0 milestone Dec 11, 2025
@codecov
Copy link

codecov bot commented Dec 11, 2025

Codecov Report

❌ Patch coverage is 69.56522% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.26%. Comparing base (8d685da) to head (71e1084).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
sdp.go 69.56% 5 Missing and 2 partials ⚠️

❌ Your patch status has failed because the patch coverage (69.56%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3297      +/-   ##
==========================================
- Coverage   84.30%   84.26%   -0.05%     
==========================================
  Files          80       80              
  Lines        9133     9144      +11     
==========================================
+ Hits         7700     7705       +5     
- Misses       1014     1019       +5     
- Partials      419      420       +1     
Flag Coverage Δ
go 84.26% <69.56%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

@philipch07 philipch07 force-pushed the pch07/permissive-sdps-except-all-invalid branch from f821646 to b6524bd Compare December 11, 2025 05:26
Copy link
Member

@JoTurk JoTurk left a comment

Choose a reason for hiding this comment

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

Pretty cool change, and I like the early return.

@philipch07 philipch07 force-pushed the pch07/permissive-sdps-except-all-invalid branch from b6524bd to 71e1084 Compare December 11, 2025 05:35
@philipch07 philipch07 merged commit db10b6f into master Dec 11, 2025
21 of 22 checks passed
@philipch07 philipch07 deleted the pch07/permissive-sdps-except-all-invalid branch December 11, 2025 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Don't reject offers/replies that contain a malformed candidate

2 participants