Skip to content

TEL-730: Add validation method variants that include soft errors.#1667

Merged
alexfish8 merged 3 commits into
mainfrom
afish/soft-errors
Jul 15, 2026
Merged

TEL-730: Add validation method variants that include soft errors.#1667
alexfish8 merged 3 commits into
mainfrom
afish/soft-errors

Conversation

@alexfish8

@alexfish8 alexfish8 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

This PR adds ValidateResult variants of some of the existing SIP Validate methods. Logically, these are the same as the current ones, except they return a ValidationResult instead of an error, where said ValidationResult is composed of an error and zero or more "soft validation errors".

Context: for certain kinds of validation failures, we'd like to log something, but swallow the error, so that the surrounding processing might still continue (we call these "soft errors"). There already exists a mechanism for logging these "soft" errors (SoftFailureReporter), but the SoftFailure reporter is global, and so it is cumbersome to pass the SoftFailure reporter request-scoped logging context (note: I had originally experimented with extending the SoftFailureReporter function signature to include a Context and adding flavors of the Validate methods that had a Context parameter, but this felt a bit error-prone).

Now, if callers want to log soft validation errors, they may do so by invoking theValidateResult variant of the Validate method and logging the soft errors of the returned value.

General notes:

  • Initially, when I wrote this, I had added variants of Validate that returned an []error rather than a ValidationResult (where the error slice contained both hard and soft errors). This was simpler in some ways (avoided introducing a new type), but it became a bit annoying to continually re-check for hard errors in various contexts. This motivated me to add a new type with a simple OK() method that could allow callers to conveniently check whether or not they should continue processing or not.
  • In the future, we might consider extending ValidationResult to contain a slice of hard errors. For now, I've opted to keep behavioral changes to a minimum, so that the validation logic will continue to eagerly return if a hard error is encountered.
  • Added various utility methods for logging and combining ValidationResults (thanks @alexlivekit for the LogSoftErrors suggestion!).

Example of how these methods will be used

@changeset-bot

changeset-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5c42903

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
github.com/livekit/protocol Minor
@livekit/protocol Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@CLAassistant

CLAassistant commented Jul 14, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@alexfish8 alexfish8 force-pushed the afish/soft-errors branch from 0d0eecd to abdbad6 Compare July 14, 2026 23:27
@alexfish8 alexfish8 changed the title TEL-730: Update validation methods to return soft errors. TEL-730: Add validation method variants that include soft errors. Jul 14, 2026
@alexfish8 alexfish8 requested a review from alexlivekit July 14, 2026 23:32
@alexfish8 alexfish8 force-pushed the afish/soft-errors branch from abdbad6 to 177344e Compare July 14, 2026 23:58
@alexfish8 alexfish8 force-pushed the afish/soft-errors branch from 177344e to 5453c4d Compare July 14, 2026 23:59

@alexlivekit alexlivekit left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a few nits, but overall lgtm

Comment thread livekit/sip_validation.go Outdated
Comment thread livekit/sip_validation.go Outdated
@alexfish8

Copy link
Copy Markdown
Contributor Author

a few nits, but overall lgtm

Thanks for the review!

@alexfish8 alexfish8 merged commit aae1fc9 into main Jul 15, 2026
8 checks passed
@alexfish8 alexfish8 deleted the afish/soft-errors branch July 15, 2026 00:28
@github-actions github-actions Bot mentioned this pull request Jul 15, 2026
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.

3 participants