Skip to content

TEL-730: Update various SIP validation methods to return error slices.#1666

Closed
alexfish8 wants to merge 2 commits into
mainfrom
afish/context-logging
Closed

TEL-730: Update various SIP validation methods to return error slices.#1666
alexfish8 wants to merge 2 commits into
mainfrom
afish/context-logging

Conversation

@alexfish8

@alexfish8 alexfish8 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Motivation: we'd like like to get a better sense as to which clients are sending invalid REFER messages (see TEL-730). In order to do so, we're updating various SIP validation methods to return slices of errors. Some of these errors might be "soft errors" (i.e., encountering them shouldn't cause the surrounding request to fail). Previously, these soft errors were could be iterated over via a global SoftFailureNotifier. The global nature of this notifier made it cumbersome to log these soft errors with request-scoped loggers. This PR updates the validation methods to include these soft errors in their return values so that callers may log them as they please.

A few additional notes:

  • In general, I've tried to avoid updating err return values to []error unless I needed to do so.
  • Additionally, so as to avoid introducing unintended behavior changes, I've tried to keep all return paths consistent in functions where I've updated the return type signature (i.e., if, previously, an err was returned eagerly, now, we'd eagerly return an error slice consisting of just that error). In the future, we might consider updating some of these methods to return a collection of all validation failures, but, I'd like to avoid introducing a lot of changes all at once.
  • Add a few helper functions to extract "hard errors" from these error slices (thanks @alexlivekit for the suggestion on LogSoftErrors).

TODO: Maintain backwards compatibility so that we don't have to do a major version bump.

@changeset-bot

changeset-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ea70639

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a 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 closed this Jul 14, 2026
@alexfish8

Copy link
Copy Markdown
Contributor Author

Closed in favor of #1667.

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