Typed throws#988
Draft
pblazej wants to merge 28 commits into
Draft
Conversation
When the WebRTC peer connection went to .disconnected/.failed, the transport delegate called primaryTransportConnectedCompleter.reset(), which threw LiveKitError(.cancelled) to anyone awaiting the completer. Consumers couldn't distinguish a network-induced disconnect from a user-initiated cancel through room.disconnectError or didDisconnectWithError. - AsyncCompleter.reset(throwing:) accepts an optional Error and normalizes via LiveKitError.from. WaitEntry.cancel(throwing:) takes an optional LiveKitError, defaulting to .cancelled, so the withTaskCancellationHandler.onCancel path still produces .cancelled for real Task cancellation. - Room+TransportDelegate now resets the transport completers with a typed LiveKitError(.network, message:) on PC disconnect/failed. - Room.cleanUp / SignalClient.cleanUp thread the disconnect cause through to in-flight completer waiters instead of masking it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends the previous .network-typing fix to the completers that the initial change missed: - CompleterMapActor.reset(throwing:) mirrors AsyncCompleter; SignalClient now passes disconnectError to _addTrackCompleters so a sendAddTrack waiter caught mid-disconnect sees the underlying cause. - DataChannelPair.reset(throwing:) + Room.cleanUpRTC(withError:) so the publisherDataChannel.openCompleter waiter inside Room.send(dataPacket:) sees .network instead of .cancelled when the room is being torn down. Adds CompleterMapActorTests covering fan-out + default-to-cancelled behavior, and promotes waitForRegistration to file scope so both suites share one helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the only completer that previously had no error-propagation path
on disconnect. Both halves of this change are needed; either alone
leaves the leak open.
- Room.cleanUp(withError:) now resets activeParticipantCompleters with
the typed disconnectError, so any RemoteParticipant.waitUntilActive
in flight is unblocked immediately with .network (or whatever cause)
instead of waiting up to its full 10s timeout.
- CompleterMapActor.resume(throwing:, for:) is now a no-op when no
completer exists for the key. cleanUpParticipants mutates each
participant's state to .unknown, which fires a fire-and-forget
Task { resume(throwing: .participantRemoved, ...) } from the
state observer in Participant.swift. Without this guard, those
Tasks land after the reset and re-populate the just-cleared map
with stale .failure(.participantRemoved) entries, which then
greet the next session's same-identity waitUntilActive.
resume(returning:, for:) keeps its auto-create behavior: success
is meaningfully sticky ("the participant is active, anyone asking
later should know"), failure is not.
Tests refactored to a do/catch + Issue.record helper because
#expect(throws:) returns Void (not E?) on the swift-testing bundled
with Xcode 16.2. New CompleterMapActorTests cover the asymmetry
(no-op on missing key for throws, remember-success for returning,
existing-waiter still receives throws).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The compiler's "never mutated" analysis flags `weak var weakRP: ... = remoteParticipant` because the runtime nil-out isn't user-code mutation — but its suggested fix (`let`) won't compile, since `weak let` is illegal. Move `weak` into the closure capture list instead. The closure now holds the only weak reference and reads it when the leak check fires; same semantics, no warning, fewer lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stopwatch is a deprecated typealias for Span and split(label:) was renamed to record(_:at:). Update PublishDeviceOptimization tests accordingly and rename the local sw → span to match the type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rror) AsyncCompleter only ever fails with LiveKitError (cancelled, timed out, or whatever resume(throwing:) is called with). Narrow the internal Result type and the public wait() signature to throws(LiveKitError) so typed throws can propagate through the completer. Room.sid() is the first leaf adopter; @objcMembers + typed-throws is fine because LiveKitError is NSError-bridgeable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Warns on public/open throwing methods that don't declare a typed throws clause. Excludes modules whose public surface is @objcMembers (DataStream, Broadcast, Audio, Token protocols, Agent chat) where typed throws is either incompatible or constrained by protocol requirements. Severity is `warning` for now; will tighten to `error` once the rest of the typed-throws migration lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wrap Foundation/network errors at the I/O edges so internal call paths through HTTP and RegionManager throw only LiveKitError. Add a typed checkCancellation() helper for use inside throws(LiveKitError) contexts. - HTTP.requestValidation: wrap URLSession.data errors as LiveKitError(.network, internalError:) and replace the bad-response URLError with LiveKitError(.network). - RegionManager: narrow resolveBest, requestSettingsIfNeeded, fetchRegionSettings, parseRegionSettings to throws(LiveKitError); wrap the URLSession.data error and the awaited Task.value (Task itself remains <Success, Error> because the stdlib has no typed Failure init). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Convert the run() boundary so callers see typed throws while the inner block stays untyped. The block can still throw arbitrary errors; the catch tail downcasts LiveKitError directly and routes anything else (including CancellationError from try Task.checkCancellation()) through LiveKitError.from. This unlocks typed throws for Track.start/stop and LocalParticipant.publish without requiring their internal _publish/startCapture chains to be narrowed first. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the file-glob excludes from .swiftlint.yml in favor of inline // swiftlint:disable:next public_typed_throws annotations on the methods that intentionally stay untyped, with a one-line comment per site explaining why (audio APIs forwarding AVAudio errors, data-stream @objcMembers forwarding StreamError, Token protocol conformances, Codable witness, etc.). Also tighten the rule's regex to ignore "throws" appearing inside parameter types like (inout State) throws -> Result, so StateSync's mutate/read no longer false-positives. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Annotate Participant.requireRoom/requireIdentity, TrackPublication.requireParticipant, SignalClient.requireWebSocket, and Room+Engine.requirePublisher with throws(LiveKitError). Each was already throwing only LiveKitError; these are pure annotation changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Track.start/stop/_mute/_unmute now declare throws(LiveKitError); the SerialRunner boundary handles error normalization, so the bodies stay the same. Drop unused throws on the internal Track.enable/disable helpers — they have no try sites and never threw, so the change is local cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@objc) DeviceManager.devices() and multiCamCompatibleDevices() narrow to throws(LiveKitError) — both already throw only LiveKitError via the underlying AsyncCompleters. VideoCapturer.startCapture/stopCapture/restartCapture stay untyped: they are explicitly @objc and Swift forbids typed throws on @objc methods (NSError** bridging can't carry the type info). Same for the @objc methods on CameraCapturer (captureDevices, canSwitchPosition, switchCameraPosition, set(cameraPosition:), set(options:), and the startCapture/stopCapture overrides of @objc methods on CameraCapturer, ARCameraCapturer, InAppCapturer, MacOSScreenCapturer). LocalAudioTrack.mute/unmute and LocalVideoTrack.mute/unmute also stay untyped — they conform to the @objc protocol LocalTrackProtocol whose requirement is @objc and therefore not typed-throws-compatible. All exceptions get an inline swiftlint:disable:next public_typed_throws with a one-line comment explaining why. Tighten the SwiftLint regex to also match `override public func ...` and related modifier orderings so the rule fires on those declarations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rows LocalTrackPublication.mute/unmute and RemoteTrackPublication.set(subscribed/ enabled/preferredFPS/preferredDimensions/videoQuality:) now declare throws(LiveKitError). They flow through TrackPublication.requireParticipant, Participant.requireRoom, and SignalClient send helpers, all of which already throw only LiveKitError. Narrow the relevant SignalClient send methods (_sendRequest, sendUpdateTrackSettings, sendUpdateVideoLayers, sendUpdateSubscription, sendUpdateSubscriptionPermission, sendUpdateParticipant) and the internal RemoteTrackPublication helpers (checkUserCanModifyTrackSettings, send(trackSettings:)). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…g layer LocalParticipant.publish (audioTrack/videoTrack/data:), unpublish, set (metadata/name/attributes:), and setTrackSubscriptionPermissions now declare throws(LiveKitError). Supporting layer: - Room+Engine.publisherShouldNegotiate, send(userPacket:), send(dataPacket:), inner ensurePublisherConnected. - Transport.remove(track:) — internal, was already throwing only LiveKitError. - DataChannelPair.send(userPacket:), send(dataPacket:), withEncryption — protobuf serializedData is wrapped as LiveKitError(.failedToConvertData), the publish continuation is downcast since all internal resume sites already use LiveKitError. - Track.onUnpublish drops its unused throws (no try sites in the body). MockDataChannelPair test override is updated to match the typed signature. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…yped (Obj-C) prepareConnection narrows to throws(LiveKitError); the call tree was already covered by Wave 1 (HTTP/RegionManager). Room.connect stays untyped: typed throws on async methods exposed via @objcMembers strips the auto-generated -connectWithUrl:token:connectOptions:roomOptions:completionHandler: selector, breaking the existing Obj-C interop that Tests/LiveKitObjCTests depends on. Suppress with a comment explaining the bridging constraint. Supporting Utils narrowed (buildUrl, buildJoinRequestUrl, toValidateUrl, buildWrappedJoinRequest); the protobuf serializedData calls inside buildWrappedJoinRequest are wrapped as LiveKitError(.failedToConvertData). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move all swiftlint:disable annotations onto the same line as the func declaration (trailing :this) so they don't orphan the doc comment that sits directly above. The explanatory regular comment goes above the doc comment, preserving the doc/declaration adjacency that swiftlint expects. Promote severity from warning to error now that all known sites are either typed or carry an inline suppression with a one-line reason. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Most audio entry points already converged on LiveKitError internally (checkAdmResult maps WebRTC ADM codes; updateRequirements wraps AVAudioSession errors as LiveKitError(.audioSession)). Just annotate the public surface and the call paths feeding it: - AudioManager: setVoiceProcessingEnabled, setManualRenderingMode, setRecordingAlwaysPreparedMode, startLocalRecording, stopLocalRecording, setEngineAvailability, acquireSessionRequirement, checkAdmResult. - AudioManager.SessionRequirementHandle.release/releaseIfNeeded — public API throws(LiveKitError); the stored release closure stays untyped because typed-throws function-type runtime support requires macOS 15+. - AudioSessionEngineObserver: acquire, set/removeRequirement, updateRequirements (typed-throws closure parameter via state.mutate block annotation). - LocalAudioTrack.startCapture/stopCapture (overrides), and LocalAudioTrackRecorder.start. - PreConnectAudioBuffer.startRecording. Wrap raw AVFoundation NSError at the I/O edges so the rest of the chain can be typed: - SoundPlayer.decodeBuffer wraps AVAudioFile reading errors as LiveKitError(.soundPlayer, internalError:). - SoundPlayer.reconnectEngine wraps AVAudioEngine.start as LiveKitError(.audioEngine, internalError:). - AudioMixRecorder.start, AudioPlayerRenderer.start: same. - AVAudioPlayerNodePool.play wraps DispatchQueue.sync's untyped rethrows via a do/catch tail. Public methods now typed: SoundPlayer.prepare, SoundPlayer.SoundHandle.play, AudioMixRecorder.start, AudioPlayerRenderer.start. Test test-doubles (TestAudioTrack, TrackTests' inner subclass) updated to match the narrowed override signatures. Suppression count drops from 47 → 32. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pattern for keeping Obj-C interop while exposing typed throws to Swift: @nonobjc public func connect(...) async throws(LiveKitError) { ... } @available(swift, obsoleted: 1.0, message: "...") @objc(connectWithUrl:token:connectOptions:roomOptions:completionHandler:) public func _objc_connect(...) async throws { try await connect(...) } The @nonobjc Swift-named method is the typed one; the @objc bridge is hidden from Swift via @available(swift, obsoleted: 1.0). The bridge preserves -connectWithUrl:...:completionHandler: that the Obj-C tests (LiveKitObjCTests) depend on. Add LiveKitError(from:) convenience init that handles pass-through for existing LiveKitError, classification (cancelled/network) for known sources, and .unknown wrap for everything else. Replaces the verbose `catch let as LiveKitError; catch ... from(error: error) ?? ...` pattern across Room, RegionManager, DataChannelPair, AudioManager, AsyncCompleter, SerialRunnerActor, WebSocket. The pattern collapses to: } catch { throw LiveKitError(from: error) } Replace `try Task.checkCancellation()` with `try checkCancellation()` (typed) throughout the connect chain (Room, Room+Engine, Room+Region, SignalClient, LocalParticipant). Convert remaining `if error is CancellationError { throw error }` re-raises to throw LiveKitError(.cancelled) instead, per the decision to wrap cancellation everywhere. Narrow WebSocket.init to throws(LiveKitError) — its delegate already resumes the continuation only with LiveKitError values. Note: signalClient.connect stays untyped because narrowing it triggers a swiftc SIL ownership crash in fullConnectSequence (interaction with the where-clause catch). The Room.connect wrapper still catches and funnels through LiveKitError(from:), so user-visible behavior is the same. Suppressions: 47 → 32 unchanged; same count, simpler code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Convert VideoCapturer (start/stop/restartCapture) and the @objc-annotated methods on CameraCapturer (captureDevices, canSwitchPosition, switchCameraPosition, set(cameraPosition:), set(options:)) to pure-Swift typed-throws methods marked @nonobjc, paired with @objc bridges hidden from Swift via @available(swift, obsoleted: 1.0). The bridges preserve the originally auto-generated Obj-C selectors (-startCaptureWithCompletionHandler:, -setCameraPosition:completionHandler:, etc.). Subclass overrides on CameraCapturer, ARCameraCapturer, InAppCapturer, MacOSScreenCapturer narrow to throws(LiveKitError). Where the override calls into raw AVFoundation/ReplayKit/ScreenCaptureKit/ARKit APIs (RTCCameraVideoCapturer.startCapture, RPScreenRecorder.startCapture, SCStream.start/stopCapture, ARKitSession.run), the NSError result is wrapped as LiveKitError(.webRTC, internalError:) or LiveKitError(.deviceAccessDenied, internalError:). Local{Audio,Video}Track.mute/unmute keep their suppressions: they satisfy the @objc LocalTrackProtocol requirement, and per the project decision protocols stay non-breaking. Suppression count drops from 32 → 24. The 8 newly-introduced bridge-method suppressions are tracked alongside the 4 protocol-conformance ones; the rest live in Token / DataStream / Broadcast modules whose constraints (protocol conformance, Codable, @objcMembers data-stream readers/writers) prevent typed throws. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduce LiveKitErrorType.dataStream (1201) with description "Data stream error". Wire LiveKitError(from:) to recognize StreamError and wrap it as LiveKitError(.dataStream, internalError:), preserving the original StreamError as the underlying error. Apply the @nonobjc + @objc-bridge pattern to the DataStream public surface: - ByteStreamWriter.write/close - TextStreamWriter.write/close - ByteStreamReader.readAll/writeToFile - TextStreamReader.readAll - PreConnectAudioBuffer.sendAudioData Each public Swift method is now throws(LiveKitError); StreamError (and AsyncFileStream.Error in writeToFile) flows through LiveKitError(from:) at the catch boundary. The companion @objc bridges (-write:completionHandler:, -closeWithReason:completionHandler:, -readAllWithCompletionHandler:, -writeToFileInDirectory:name:completionHandler:) preserve the auto-generated Obj-C selectors that LiveKitObjCTests/DataStreamObjCTests.m depends on. Suppression count: 24 → 23. The reduction is small because each bridge method itself carries an intentional suppression, but the *public Swift API* surface for data streams is now fully typed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The bridge methods we added for Obj-C interop (named _objc_*, hidden from Swift via @available(swift, obsoleted: 1.0)) had to carry inline swiftlint:disable annotations because @objc forbids typed throws. They are not first-class Swift API though — Swift consumers see only the typed @nonobjc method that the bridge wraps. Tighten the regex to skip func names starting with _objc_, then strip the now-redundant inline suppressions across VideoCapturer, CameraCapturer, ByteStreamReader/Writer, TextStreamReader/Writer, and Room. Suppression count: 23 → 7. Remaining suppressions are the genuinely constrained ones: - LocalAudioTrack/LocalVideoTrack mute/unmute (4) — @objc LocalTrackProtocol - LiteralTokenSource/CachingTokenSource fetch (2) — protocol conformance - BroadcastAudioCodec encode (1) — Encodable witness Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The split was a leftover from when connect had to wrap _connect's untyped
throws in a do/catch. Now that the body's pre-conditions throw LiveKitError
directly and the existing inner catch already runs cleanup before re-throwing,
we just convert there:
} catch {
// cleanup
await cleanUp(withError: error)
throw LiveKitError(from: error) // was: throw error
}
The function declares throws(LiveKitError) directly. Less indirection;
identical behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Track.swift: narrow startCapture/stopCapture base class (empty default impls) to throws(LiveKitError); drop unused throws on onPublish (no try sites in body, no callers). - LocalVideoTrack: narrow startCapture/stopCapture overrides — they delegate to capturer.start/stopCapture which are already typed. - LocalAudioTrack: narrow AudioFrameWatcher.wait (calls completer.wait, typed) and startWaitingForFrames. - AsyncSerialDelegate.notifyAsync: narrow to throws(LiveKitError); the closure parameter is non-throwing so SerialRunnerActor's typed-throws contract is satisfied. - WebSocket: collapse `LiveKitError.from(error: error) ?? error` and `LiveKitError.from(error: error) ?? LiveKitError(.unknown)` to `LiveKitError(from: error)` — error is non-nil at both sites so the fallback was dead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tier 1 (annotation-only, body throws only LiveKitError): - LocalTrackPublication.suspend/resume — call typed mute/unmute - RemoteParticipant.waitUntilActive — uses requireRoom/Identity + completer - Transport.add(iceCandidate:) — drop throws (no try sites) - SignalClient.send(offer:offerId:), send(answer:offerId:), sendMuteTrack — bodies just call typed _sendRequest Tier 2 (wrap one continuation, otherwise typed): - Transport.set(remoteDescription:[offerId:]), set(localDescription:), createOffer, createAnswer, createAndSendOffer, _negotiateSequence — WebRTC NSError from peer-connection callbacks gets wrapped at the resume site as LiveKitError(.webRTC, internalError:); the function body uses do/catch with LiveKitError(from:) as a safety net so the pre-existing LiveKitError(.invalidState) in the no-sd-no-error path passes through. Transport.OnOfferBlock typealias also narrows to throws(LiveKitError); the closure passed in Room+Engine.configureTransports gets an explicit `throws(LiveKitError)` annotation since Swift doesn't infer typed throws from closure body content. LocalAudioTrack.startWaitingForFrames stays untyped — TestAudioTrack in LiveKitTestSupport overrides it via @objc dispatch (extension method overriding only works for @objc-dispatched methods, which typed throws disables). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four #expect(throws: (any Error).self) sites pinned to LiveKitError.self since the underlying APIs now declare throws(LiveKitError): - RemoteParticipantTests.swift:42 — RemoteParticipant.waitUntilActive - AudioEnginePermission.swift:39 — AudioManager.startLocalRecording - AudioEngineObserver.swift:65 — AudioManager.stopLocalRecording - AudioEngineObserver.swift:76 — AudioManager.startLocalRecording Better diagnostics if anything other than a LiveKitError is ever thrown. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extend the Error Handling section with the typed-throws conventions and the constraints we hit during the migration: - public_typed_throws rule + LiveKitError(from:) at I/O edges - typed checkCancellation() helper - @objc + typed throws incompatibility → @nonobjc + _objc_ shim pattern with @available(swift, obsoleted: 1.0) - @objc protocol conformers stay untyped (decision 4) - Task<_, Failure: Error> and withCheckedThrowingContinuation lack typed-failure initializers — convert at await site - Stored typed-throws closures need macOS 15+ - Protocol witnesses can narrow throws (Codable etc.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Six bullets → four. Drop the protocol-witness narrowing one (already implied by the broader narrowing guidance) and merge the Task and continuation bullets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.