Skip to content

Migrate enum conversions from map lookups to switch expressions - #1156

Merged
hiroshihorie merged 6 commits into
mainfrom
hiroshi/degradation-preference-disabled-crash
Aug 7, 2026
Merged

Migrate enum conversions from map lookups to switch expressions#1156
hiroshihorie merged 6 commits into
mainfrom
hiroshi/degradation-preference-disabled-crash

Conversation

@hiroshihorie

@hiroshihorie hiroshihorie commented Aug 7, 2026

Copy link
Copy Markdown
Member

Cleans up how we convert between our enums and their WebRTC or protobuf counterparts. All 14 conversion sites (extensions.dart, track/options.dart, support/native_audio.dart) move from the {...}[this]! map lookup pattern to switch expressions.

Why

Switch expressions are the better tool for this in modern Dart:

  • Over real Dart enums the switch is compile time exhaustive with no wildcard arm. If an enum gains a value, the analyzer flags every conversion that needs updating, instead of the gap only showing up at runtime.
  • Over protobuf enums (which are classes, so exhaustiveness cannot be proven) the switch keeps an explicit _ arm with a sensible default. Values that a newer server sends and an older SDK does not know about now degrade gracefully, for example an unrecognized DisconnectReason in a leave request maps to DisconnectReason.unknown.
  • No intermediate map is allocated per call.

Mapping correction for the deprecated disabled

While converting DegradationPreferenceExt, the deprecated DegradationPreference.disabled (previously absent from the map) now maps to MAINTAIN_FRAMERATE_AND_RESOLUTION, which is exactly how upstream WebRTC defines it (api/rtp_parameters.h):

MAINTAIN_FRAMERATE_AND_RESOLUTION,
// TODO(webrtc:450044904): Switch downstream projects to
// MAINTAIN_FRAMERATE_AND_RESOLUTION and remove DISABLED.
DISABLED = MAINTAIN_FRAMERATE_AND_RESOLUTION,

The deprecation message now points users to maintainFramerateAndResolution. Removing the value entirely is left for the next major, mirroring upstream's own TODO.

Follow-up (separate PR): add public DisconnectReason members for the newer proto reasons so apps can distinguish them from genuinely unknown.

Tests

New test/types/degradation_preference_rtc_test.dart asserts every DegradationPreference value converts cleanly and that disabled maps to MAINTAIN_FRAMERATE_AND_RESOLUTION. Full suite passes (378 tests), analyze, format, and import sorter clean.

🤖 Generated with Claude Code

toRTCType built a map without an entry for disabled and force unwrapped
the lookup, so an explicit disabled preference threw a null check error
during publish. WebRTC has renamed DISABLED to
MAINTAIN_FRAMERATE_AND_RESOLUTION and defines the old name as an alias,
so map it accordingly. The conversion is now an exhaustive switch, which
turns any future enum addition into a compile error instead of a runtime
crash.
The map-with-null-assertion pattern crashes at runtime when a value is
missing from the map, which is how the DegradationPreference.disabled
bug happened. Switch expressions over Dart enums are compile time
exhaustive, so a future enum addition becomes an analyzer error instead.

Protobuf enums are classes rather than Dart enums, so their switches
keep a wildcard arm. Where the old code force unwrapped the lookup, the
wildcard now falls back to a safe default instead of crashing on values
from newer servers. DisconnectReason was already affected in practice,
the proto defines 17 reasons but only 8 were mapped, so a server sending
ROOM_CLOSED or MIGRATION in a leave request crashed disconnect handling.
@hiroshihorie hiroshihorie changed the title Fix DegradationPreference.disabled crash and migrate enum conversions to switch expressions Migrate enum conversions from map lookups to switch expressions Aug 7, 2026
@hiroshihorie
hiroshihorie marked this pull request as ready for review August 7, 2026 10:48

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

toRid had no callers and mapped VideoQuality.OFF to the low quality rid,
which would be wrong if it ever gained one. The new test pins the value
count of every converted protobuf enum, so a proto regen that adds a
value fails the test and points at the conversion, since the wildcard
arms mean the analyzer can no longer flag it.
@hiroshihorie
hiroshihorie merged commit 3f67c74 into main Aug 7, 2026
14 checks passed
@hiroshihorie
hiroshihorie deleted the hiroshi/degradation-preference-disabled-crash branch August 7, 2026 11:26
hiroshihorie added a commit that referenced this pull request Aug 8, 2026
…values (#1158)

Follow-up to #1156 (replaces #1157, which GitHub auto-closed when the
stack base merged).

The protocol defines 17 disconnect reasons but the public
`DisconnectReason` enum only had members for 8 of them, so newer reasons
arriving in a leave request (for example when the server closes a room
or a SIP trunk fails) were all collapsed to `unknown`. This adds members
for the nine missing reasons and maps them in `toSDKType`:

`migration`, `signalClose`, `roomClosed`, `userUnavailable`,
`userRejected`, `sipTrunkFailure`, `connectionTimeout`, `mediaFailure`,
`agentError`

Unrecognized values from servers newer than the SDK still fall back to
`unknown` via the wildcard arm introduced in #1156.

Note for apps: adding enum members means an exhaustive `switch` over
`DisconnectReason` in app code will need new cases, which is why the
changeset is `minor`.

## Tests

New `test/types/disconnect_reason_test.dart` asserts every proto value
maps to a distinct SDK value and pins the nine new mappings. Full suite
passes (380 tests), analyze, format, and import sorter clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.

1 participant