-
Notifications
You must be signed in to change notification settings - Fork 1.2k
WebRTC Client. Make IceServer.urls a list
#5253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughWebRtc.IceServer Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| */ | ||
| public data class IceServer( | ||
| val urls: String, | ||
| val urls: List<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you introduce an overload constructor to mitigate the breaking change here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that should work. Forgot about it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt (2)
7-8: Consider using star imports per project guidelines.The coding guidelines specify using star imports for
io.ktor.*packages. Consider refactoring to:import io.ktor.client.webrtc.* import io.ktor.utils.io.*As per coding guidelines, this aligns with the project's import conventions.
10-10: Use modern @OptIn syntax.The
markerClassparameter form is outdated. Modern Kotlin uses the simplified syntax:-@OptIn(markerClass = [ExperimentalKtorApi::class]) +@OptIn(ExperimentalKtorApi::class)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtc.ktktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcPeerConnection.ktktor-client/ktor-client-webrtc/gradle.propertiesktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.ktktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.ktsktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/src/io/ktor/client/webrtc/rs/Engine.ktktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/gradle.properties
🚧 Files skipped from review as they are similar to previous changes (1)
- ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtc.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.ktktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/src/io/ktor/client/webrtc/rs/Engine.ktktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcPeerConnection.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.kt
**/test/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Follow test naming pattern:
DescribeWhatIsBeingTested
Files:
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.kt
🧠 Learnings (19)
📚 Learning: 2025-08-14T15:17:11.466Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts:12-12
Timestamp: 2025-08-14T15:17:11.466Z
Learning: The Gobley Cargo plugin (dev.gobley.cargo) used in the Ktor WebRTC RS module requires the kotlin("plugin.atomicfu") plugin as a dependency, so the atomicfu plugin should not be removed even if there are no direct kotlinx.atomicfu imports in the module's source code.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts
📚 Learning: 2025-08-14T15:17:11.466Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts:12-12
Timestamp: 2025-08-14T15:17:11.466Z
Learning: The Cargo plugin (dev.gobley.cargo) used in the Ktor WebRTC RS module depends on the kotlin("plugin.atomicfu") plugin, so atomicfu should not be removed even if there are no direct kotlinx.atomicfu imports in the module's source code.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.ktsktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/gradle.propertiesktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt
📚 Learning: 2025-08-14T15:17:11.466Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts:12-12
Timestamp: 2025-08-14T15:17:11.466Z
Learning: The Gobley Gradle plugin (dev.gobley.cargo and dev.gobley.uniffi) requires the kotlin("plugin.atomicfu") plugin for atomic types support in Rust-Kotlin bindings generated by UniFFI. The atomicfu plugin should not be removed even if there are no direct kotlinx.atomicfu imports in the Kotlin source code, as it's essential for proper Rust-Kotlin interoperability.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts
📚 Learning: 2025-07-01T10:54:53.751Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4970
File: build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts:67-72
Timestamp: 2025-07-01T10:54:53.751Z
Learning: In build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts, the KT-70915 and KT-77609 workarounds that limit parallelism for KotlinNativeLink and CInteropCommonizerTask should remain unconditional (not gated behind CI flag) as these limits are beneficial for both local and CI builds.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.ktsktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/gradle.propertiesktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.ktsktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/gradle.propertiesktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-11-27T10:07:26.686Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420
Timestamp: 2025-11-27T10:07:26.686Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.ktsktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/gradle.properties
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.ktsktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/gradle.propertiesktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts
📚 Learning: 2025-04-23T11:48:32.634Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4767
File: build-settings-logic/src/main/kotlin/ktorsettings.kotlin-user-project.settings.gradle.kts:136-146
Timestamp: 2025-04-23T11:48:32.634Z
Learning: For Kotlin Train builds in the Ktor project, the properties `atomicfu_version`, `coroutines_version`, and `serialization_version` are expected to be defined in the TeamCity CI environment, so using `.get()` without custom error handling is sufficient.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts
📚 Learning: 2025-08-15T09:12:47.243Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/rust/senders.rs:88-96
Timestamp: 2025-08-15T09:12:47.243Z
Learning: In the webrtc Rust crate version used in the Ktor WebRTC RS implementation, the RTCRtpCodingParameters struct's rtx field is NOT optional and can be accessed directly as params.rtx.ssrc without needing Option handling.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.ktsktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Use star imports for `io.ktor.*` packages
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: In Ktor test files, particularly in the ktor-client/ktor-client-core/jvm/test/ directory, test files follow the convention of not including explicit package declarations. This is consistent across test files like CachingCacheStorageTest.kt and should be maintained for consistency.
Applied to files:
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.kt
📚 Learning: 2025-05-19T14:56:46.265Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4861
File: teamcity.default.properties:5-7
Timestamp: 2025-05-19T14:56:46.265Z
Learning: The `kotlin.native.jvmargs` property should be kept separate from the `org.gradle.jvmargs` and from the `GRADLE_OPTS` environment variable in CI configuration.
Applied to files:
ktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-05-14T15:56:33.598Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4856
File: teamcity.default.properties:5-5
Timestamp: 2025-05-14T15:56:33.598Z
Learning: The Ktor project runs binary-compatibility-validator tasks on CI which require substantial metaspace, so adding metaspace size limits (-XX:MaxMetaspaceSize) to Gradle options would cause these tasks to fail.
Applied to files:
ktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: This is a multiplatform project (JVM, JS, Native), and platform-specific considerations should be respected.
Applied to files:
ktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Project is multiplatform (JVM, JS, Native) with tests organized by platform in module's test directories
Applied to files:
ktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/src/io/ktor/client/webrtc/rs/Engine.kt
📚 Learning: 2025-09-30T07:52:14.769Z
Learnt from: yschimke
Repo: ktorio/ktor PR: 4013
File: ktor-client/ktor-client-android/jvm/src/io/ktor/client/engine/android/Android14URLConnectionFactory.kt:24-26
Timestamp: 2025-09-30T07:52:14.769Z
Learning: In the Ktor Android HTTP client engine (ktor-client-android), prefer using `URI.create(urlString).toURL()` over the `URL(urlString)` constructor when opening connections with Android's HttpEngine, as it avoids deprecated APIs and the different exception behavior (IllegalArgumentException vs MalformedURLException) is acceptable.
Applied to files:
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/src/io/ktor/client/webrtc/rs/Engine.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Mark internal APIs with `InternalAPI` annotation
Applied to files:
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.kt
🔇 Additional comments (9)
ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/src/io/ktor/client/webrtc/rs/Engine.kt (1)
36-36: LGTM! IceServer construction correctly updated.The change properly reflects the updated
IceServerAPI whereurlsis nowList<String>. Passingit.urlsdirectly (instead of wrapping inlistOf()) is correct.ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcPeerConnection.kt (1)
133-133: Verify the visibility change is intentional and doesn't break module-internal usage.The visibility modifier changed from
internaltoprotected, restricting access to subclasses only. This change appears unrelated to the PR's stated objective of makingIceServer.urlsa list. Whileprotectedis more restrictive and appropriate for this abstract class pattern, please confirm:
- This change is intentional
- No other code within the module (but outside subclasses) depends on this function
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.kt (3)
7-7: LGTM!The typealias correctly maps to JUnit 5's
@Disabledannotation, providing appropriate JVM-specific test ignore functionality.
9-10: LGTM!The annotation is correctly defined with appropriate targets (CLASS and FUNCTION) for a test utility. The actual constructor properly implements the expect declaration.
1-3: Verify the copyright year.The copyright includes 2026, but this PR was created in December 2025. Confirm this is intentional (e.g., expected merge date) or if it should be 2025.
⛔ Skipped due to learnings
Learnt from: osipxd Repo: ktorio/ktor PR: 5205 File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420 Timestamp: 2025-11-27T10:07:26.686Z Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.Learnt from: osipxd Repo: ktorio/ktor PR: 5205 File: ktor-client/ktor-client-curl/libcurl/ports/curl/dependencies.patch:156-160 Timestamp: 2025-11-27T10:07:51.060Z Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/libcurl/ports/*` as these are vcpkg port files copied from the vcpkg repository and maintained upstream, not by the Ktor team.Learnt from: rururux Repo: ktorio/ktor PR: 4896 File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12 Timestamp: 2025-05-30T06:45:52.309Z Learning: In Ktor test files, particularly in the ktor-client/ktor-client-core/jvm/test/ directory, test files follow the convention of not including explicit package declarations. This is consistent across test files like CachingCacheStorageTest.kt and should be maintained for consistency.Learnt from: rururux Repo: ktorio/ktor PR: 4896 File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12 Timestamp: 2025-05-30T06:45:52.309Z Learning: In Ktor test files, particularly in the ktor-client/ktor-client-core/jvm/test/ directory, test files follow the convention of not including explicit package declarations. This is consistent across test files like CachingCacheStorageTest.kt and should be maintained for consistency.Learnt from: CR Repo: ktorio/ktor PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-25T09:38:19.393Z Learning: Applies to **/*.kt : API changes must be intentional and well-documentedktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/gradle.properties (1)
6-7: No action required — target disablements are correctly configured.The parent module (
ktor-client-webrtc) setstarget.darwin=false, which already disables tvOS, watchOS, and all macOS variants at the module level. The child module (ktor-client-webrtc-rs) only needs to overridetarget.ios=falsesince iOS is explicitly enabled in the parent. This design correctly inherits the Darwin disablement while selectively disabling iOS for the rs module.Likely an incorrect or invalid review comment.
ktor-client/ktor-client-webrtc/gradle.properties (1)
10-11: Update the comment to accurately describe the JVM srcset purpose.The comment describes the JVM srcset as "dummy," but the
jvm/testdirectory contains actual test utilities (ConnectionUtils.jvm.ktandIgnore.jvm.kt) that provide platform-specific implementations for JVM testing. The JVM target is needed because thektor-client-webrtc-rsmodule uses the Cargo plugin to compile Rust code and generate JVM bindings via UniFFFI, which requires JVM compilation support.Suggested update:
# JVM target is required for the Cargo plugin to generate JVM bindings from the Rust libraryLikely an incorrect or invalid review comment.
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt (1)
11-13: No action needed. The test classes that callcreateTestWebRtcClient()are annotated with@IgnoreJvm, so theTODO()stub on JVM will never be executed. The implementation is intentional and safe.ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts (1)
11-14: Update or remove outdated version comment and justify using 0.3.5 instead of 0.3.7.The comment referencing Gobley plugin version 0.3.8 is misleading—version 0.3.8 has not been released. Current available versions are up to 0.3.7, yet the code uses 0.3.5. Either update the versions to use the latest 0.3.7 or document why 0.3.5 is preferred over newer releases and remove the reference to the unreleased 0.3.8.
bjhham
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
177d3b2 to
1a032f5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @ktor-client/ktor-client-webrtc/gradle.properties:
- Around line 10-11: Update the misleading comment above target.jvm=true to
explain that the JVM target is intentionally enabled to declare and track the
JVM API surface (api/jvm/) for binary compatibility and to expose JVM bindings
for the Rust variant (ktor-client-webrtc-rs), not merely as a "dummy"
placeholder; mention the existence of api/jvm/ and that this enables JVM
consumers and Rust/JNI bindings to build correctly.
🧹 Nitpick comments (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtc.kt (1)
92-108: IceServer.urls list migration and secondary constructor look correct; consider clarifying KDoc and named-arg impactThe switch to
urls: List<String>plus the single‑URL secondary constructor is a good way to support multiple ICE URLs while keeping most existing call sites working. The only remaining break is for Kotlin callers that used a named argument likeurls = "stun:..."; they’ll now need eitherurls = listOf("…")orurl = "…", which seems acceptable for an experimental API.You might want to slightly tighten the KDoc to make the collection type explicit, e.g. “List of URLs of the ICE server (e.g. STUN/TURN URIs)” and optionally mention the convenience single‑URL constructor.
As per coding guidelines, API changes here are intentional and documented.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
gradle/artifacts/publishJvmAndCommonPublications.txtktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.apiktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.apiktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.apiktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtc.ktktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcPeerConnection.ktktor-client/ktor-client-webrtc/gradle.propertiesktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.ktktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/api/android/ktor-client-webrtc-rs.apiktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.ktsktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/src/io/ktor/client/webrtc/rs/Engine.ktktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/gradle.properties
🚧 Files skipped from review as they are similar to previous changes (5)
- ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts
- ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcPeerConnection.kt
- ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/src/io/ktor/client/webrtc/rs/Engine.kt
- ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.kt
- ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/gradle.properties
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.ktktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtc.kt
**/test/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Follow test naming pattern:
DescribeWhatIsBeingTested
Files:
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt
🧠 Learnings (21)
📚 Learning: 2025-04-22T12:33:16.705Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4805
File: build-logic/src/main/kotlin/ktorbuild/internal/publish/ValidatePublishedArtifactsTask.kt:74-82
Timestamp: 2025-04-22T12:33:16.705Z
Learning: The ValidatePublishedArtifactsTask in the Ktor build system is designed to collect artifacts from all PublishToMavenRepository tasks in the task graph, not just from the specifically named publish task. This is intentional to validate all publications triggered by a task, including those from task dependencies.
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txtktor-client/ktor-client-webrtc/gradle.propertiesktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txtktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.apiktor-client/ktor-client-webrtc/gradle.propertiesktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtc.ktktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txtktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txtktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
📚 Learning: 2025-08-14T15:17:11.466Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts:12-12
Timestamp: 2025-08-14T15:17:11.466Z
Learning: The Cargo plugin (dev.gobley.cargo) used in the Ktor WebRTC RS module depends on the kotlin("plugin.atomicfu") plugin, so atomicfu should not be removed even if there are no direct kotlinx.atomicfu imports in the module's source code.
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Use star imports for `io.ktor.*` packages
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txt
📚 Learning: 2025-11-27T10:07:26.686Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420
Timestamp: 2025-11-27T10:07:26.686Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txt
📚 Learning: 2025-08-14T15:17:11.466Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts:12-12
Timestamp: 2025-08-14T15:17:11.466Z
Learning: The Gobley Cargo plugin (dev.gobley.cargo) used in the Ktor WebRTC RS module requires the kotlin("plugin.atomicfu") plugin as a dependency, so the atomicfu plugin should not be removed even if there are no direct kotlinx.atomicfu imports in the module's source code.
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txt
📚 Learning: 2025-11-27T10:07:51.060Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/libcurl/ports/curl/dependencies.patch:156-160
Timestamp: 2025-11-27T10:07:51.060Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/libcurl/ports/*` as these are vcpkg port files copied from the vcpkg repository and maintained upstream, not by the Ktor team.
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txt
📚 Learning: 2025-05-07T09:12:14.293Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4836
File: ktor-utils/build.gradle.kts:35-35
Timestamp: 2025-05-07T09:12:14.293Z
Learning: The Ktor project maintains a flat Gradle project structure (where projects are referenced without nested paths like `:ktor-test-base`) while keeping a hierarchical directory organization on disk.
Applied to files:
gradle/artifacts/publishJvmAndCommonPublications.txt
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.apiktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtc.kt
📚 Learning: 2025-08-15T08:43:28.980Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/rust/connection.rs:222-236
Timestamp: 2025-08-15T08:43:28.980Z
Learning: In WebRTC.rs (webrtc crate), remote track removal detection is limited compared to browser WebRTC APIs. The onmute event is the most reliable indicator for track removal, as transceiver direction changes after renegotiation don't happen immediately and have asynchronous timing issues. Polling solutions are not recommended due to performance concerns.
Applied to files:
ktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.apiktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
📚 Learning: 2025-07-01T10:54:53.751Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4970
File: build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts:67-72
Timestamp: 2025-07-01T10:54:53.751Z
Learning: In build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts, the KT-70915 and KT-77609 workarounds that limit parallelism for KotlinNativeLink and CInteropCommonizerTask should remain unconditional (not gated behind CI flag) as these limits are beneficial for both local and CI builds.
Applied to files:
ktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-05-19T14:56:46.265Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4861
File: teamcity.default.properties:5-7
Timestamp: 2025-05-19T14:56:46.265Z
Learning: The `kotlin.native.jvmargs` property should be kept separate from the `org.gradle.jvmargs` and from the `GRADLE_OPTS` environment variable in CI configuration.
Applied to files:
ktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: This is a multiplatform project (JVM, JS, Native), and platform-specific considerations should be respected.
Applied to files:
ktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Project is multiplatform (JVM, JS, Native) with tests organized by platform in module's test directories
Applied to files:
ktor-client/ktor-client-webrtc/gradle.properties
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: In Ktor test files, particularly in the ktor-client/ktor-client-core/jvm/test/ directory, test files follow the convention of not including explicit package declarations. This is consistent across test files like CachingCacheStorageTest.kt and should be maintained for consistency.
Applied to files:
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt
📚 Learning: 2025-08-15T09:12:47.243Z
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/rust/senders.rs:88-96
Timestamp: 2025-08-15T09:12:47.243Z
Learning: In the webrtc Rust crate version used in the Ktor WebRTC RS implementation, the RTCRtpCodingParameters struct's rtx field is NOT optional and can be accessed directly as params.rtx.ssrc without needing Option handling.
Applied to files:
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.ktktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
📚 Learning: 2025-09-05T12:47:49.016Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:35-52
Timestamp: 2025-09-05T12:47:49.016Z
Learning: Jetty's AbstractConnection class implements Closeable interface, so any class extending AbstractConnection (like JettyWebsocketConnection) can be used with Kotlin's use { } extension function for resource management.
Applied to files:
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.apiktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
📚 Learning: 2025-09-05T12:47:49.016Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:35-52
Timestamp: 2025-09-05T12:47:49.016Z
Learning: JettyWebsocketConnection in ktor-server-jetty-jakarta implements Closeable interface (has close() method), so connection.use { } is valid Kotlin syntax and will compile correctly.
Applied to files:
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api
🪛 detekt (1.23.8)
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt
[warning] 15-16: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🔇 Additional comments (12)
gradle/artifacts/publishJvmAndCommonPublications.txt (1)
182-184: The artifact publication entries forktor-client-webrtc-jvmare correctly formatted and positioned within the publication configuration. No additional API verification is required—theIceServerAPI withurlsasList<String>is already tracked in the existing API definition files at./ktor-client/ktor-client-webrtc/api/jvm/and variant directories, with backward compatibility maintained through constructor overloads.Likely an incorrect or invalid review comment.
ktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.api (2)
357-373: Android ABI for WebRtc.IceServer correctly preserves old ctor and adds List‑based URLsThe Android API surface now exposes both the old
IceServer(String, String, String)ctor and the newIceServer(List, String, String)ctor, withcomponent1,copy, andgetUrlsall usingList. That’s the expected shape for theurls: List<String>migration and should keep existing JVM/Android binaries working while enabling multiple URLs.
766-796: Android WebRtcPeerConnection.runInConnectionScope protected helper matches common APIThe added protected
runInConnectionScope(Function0)onAndroidWebRtcPeerConnection’s base (WebRtcPeerConnection) lines up with the common declaration, giving platform implementations a scoped helper without expanding the public surface for consumers.ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api (2)
164-178: runInConnectionScope is correctly surfaced in the klib ABIThe
final inline fun runInConnectionScope(Function0<Unit>)entry onWebRtcPeerConnectionis present with the expected signature, matching the common declaration and Android/JVM views. That keeps the helper available to platform subclasses without altering the public call surface for regular users.
597-615: Klib ABI for WebRtc.IceServer matches the new List URLs designThe klib dump now has:
- Primary ctor
IceServer(List<String>, String?, String?)- Secondary ctor
IceServer(String, String?, String?)urlsgetter,component1, andcopyall typed asList<String>This matches the updated Kotlin data class and mirrors the Android/JVM ABI shape. Existing native code compiled against the old
urls: Stringproperty will need to be rebuilt, but given the experimental status that’s an acceptable, intentional evolution.ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api (3)
1-200: New JVM ABI file aligns with existing common WebRTC APIsThis JVM
.apisurface mirrors the common/klib WebRTC types (data channels, tracks, connection config, stats, etc.), so it brings the JVM target under ABI tracking as required by the project’s/apiconventions. Nothing here looks out of sync with the shared definitions.
252-268: JVM ABI for WebRtc.IceServer correctly encodes both String and List‑based constructorsOn JVM,
WebRtc$IceServernow exposes:
- The existing
IceServer(String, String, String)ctor (for binary compatibility)- The new
IceServer(List, String, String)ctorcomponent1,copy, andgetUrlsreturningListThat matches the Kotlin data class change to
urls: List<String>while keeping previous binaries calling the String‑based ctor valid.
661-691: JVM WebRtcPeerConnection.runInConnectionScope is present as a protected helperThe JVM ABI for
WebRtcPeerConnectionincludes the protectedrunInConnectionScope(Function0)method, consistent with the common and Android/klib definitions. This gives platform‑specific subclasses a unified way to run code in the connection scope without expanding the public API footprint.ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt (4)
1-4: LGTM!Standard copyright header is correct.
5-8: LGTM!Package declaration and imports are appropriate.
15-16: LGTM! Empty implementation is intentional for JVM.The empty implementation is correct for the JVM platform, where audio/video permissions don't require runtime grants (unlike Android/iOS). This is a valid no-op implementation of the
actualfunction.The static analysis warning can be safely ignored.
1-16: Run linting and formatting checks.Per coding guidelines, run before finalizing:
./gradlew lintKotlin ./gradlew formatKotlin
| # jvm "dummy" srcset is needed for `ktor-client-webrtc-rs` | ||
| target.jvm=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify JVM source structure for ktor-client-webrtc
echo "=== Directory structure of ktor-client-webrtc ==="
find ktor-client/ktor-client-webrtc -type d | head -30
echo -e "\n=== Kotlin/Java source files in ktor-client-webrtc ==="
find ktor-client/ktor-client-webrtc -type f \( -name "*.kt" -o -name "*.java" \) | head -20
echo -e "\n=== gradle.properties content ==="
cat ktor-client/ktor-client-webrtc/gradle.properties
echo -e "\n=== Check for api/ files related to webrtc ==="
find . -path "./api/*" -name "*webrtc*" 2>/dev/null | head -20
echo -e "\n=== Check src directory structure ==="
ls -la ktor-client/ktor-client-webrtc/src/ 2>/dev/null || echo "src/ directory not found"
echo -e "\n=== Look for srcSets definition ==="
find ktor-client/ktor-client-webrtc -name "build.gradle.kts" -exec head -100 {} \;Repository: ktorio/ktor
Length of output: 8942
Clarify what "dummy" JVM srcset means and why it's required for the Rust component.
The comment's phrasing is misleading: while the main ktor-client-webrtc module has no actual JVM source files, the api/jvm/ directory exists, indicating that JVM API surface must be declared and tracked per binary compatibility requirements. Simply calling it "dummy" obscures this requirement.
Revise the comment to explain the actual purpose: the JVM target is enabled to expose common APIs to JVM consumers and support building the Rust variant (ktor-client-webrtc-rs) with JVM bindings, not as a placeholder.
🤖 Prompt for AI Agents
In @ktor-client/ktor-client-webrtc/gradle.properties around lines 10-11, Update
the misleading comment above target.jvm=true to explain that the JVM target is
intentionally enabled to declare and track the JVM API surface (api/jvm/) for
binary compatibility and to expose JVM bindings for the Rust variant
(ktor-client-webrtc-rs), not merely as a "dummy" placeholder; mention the
existence of api/jvm/ and that this enables JVM consumers and Rust/JNI bindings
to build correctly.
| @OptIn(markerClass = [ExperimentalKtorApi::class]) | ||
| actual fun createTestWebRtcClient(): WebRtcClient { | ||
| TODO("Not yet implemented") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usage of createTestWebRtcClient in test files
echo "=== Searching for createTestWebRtcClient usage in ktor-client-webrtc ==="
rg -n --type=kotlin 'createTestWebRtcClient' ktor-client/ktor-client-webrtc/
echo ""
echo "=== Searching for expect declaration ==="
rg -n --type=kotlin 'expect.*createTestWebRtcClient' ktor-client/ktor-client-webrtc/
echo ""
echo "=== Checking file structure of ConnectionUtils ==="
fd 'ConnectionUtils' ktor-client/ktor-client-webrtc/Repository: ktorio/ktor
Length of output: 2384
🏁 Script executed:
#!/bin/bash
# Check how createTestWebRtcClient is used in the test files
echo "=== WebRtcEngineTest.kt usage ==="
rg -n -A5 -B2 'createTestWebRtcClient' ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcEngineTest.kt | head -20
echo ""
echo "=== Check JVM implementation details ==="
cat -n ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt
echo ""
echo "=== Check if JVM tests are skipped ==="
rg -n 'skip|Skip|ignore|Ignore|@Ignore' ktor-client/ktor-client-webrtc/jvm/test/ | head -10Repository: ktorio/ktor
Length of output: 1189
🏁 Script executed:
#!/bin/bash
# Check the web implementation to understand what's needed
echo "=== Web implementation ==="
cat -n ktor-client/ktor-client-webrtc/web/test/io/ktor/client/webrtc/utils/ConnectionUtils.web.kt
echo ""
echo "=== iOS implementation ==="
cat -n ktor-client/ktor-client-webrtc/ios/test/io/ktor/client/webrtc/utils/ConnectionUtils.ios.ktRepository: ktorio/ktor
Length of output: 1917
Implement JVM WebRTC test client—the TODO blocks test execution.
The createTestWebRtcClient() function is called in common tests (WebRtcEngineTest, WebRtcDataChannelTest, WebRtcMediaTest) during @BeforeTest setup. The current TODO implementation will throw NotImplementedError and prevent JVM tests from running. All other platforms (Web, iOS, Android) have working implementations. Provide a proper JVM implementation for testing—either using an actual WebRTC backend or a mock equivalent compatible with the JVM environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The real implementation should be merged soon. Now, the tests on JVM are disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that JVM tests are disabled using the IgnoreJvm annotation
echo "=== Checking for IgnoreJvm usage in common tests ==="
rg -n '@IgnoreJvm' ktor-client/ktor-client-webrtc/common/test/
echo ""
echo "=== Checking the IgnoreJvm definition ==="
cat ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.kt
Length of output: 759
@zibet27, thanks for clarifying! I can confirm that all the WebRTC tests (WebRtcEngineTest, WebRtcMediaTest, and WebRtcDataChannelTest) are properly annotated with @IgnoreJvm, which maps to JUnit's @Disabled. This means the TODO implementation won't cause test failures on JVM. This approach makes sense as a temporary state while awaiting the real implementation. 👍
🧠 Learnings used
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/rust/senders.rs:88-96
Timestamp: 2025-08-15T09:12:47.243Z
Learning: In the webrtc Rust crate version used in the Ktor WebRTC RS implementation, the RTCRtpCodingParameters struct's rtx field is NOT optional and can be accessed directly as params.rtx.ssrc without needing Option handling.
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:35-52
Timestamp: 2025-09-05T12:47:49.016Z
Learning: Jetty's AbstractConnection class implements Closeable interface, so any class extending AbstractConnection (like JettyWebsocketConnection) can be used with Kotlin's use { } extension function for resource management.
Learnt from: bjhham
Repo: ktorio/ktor PR: 4887
File: ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyWebsocketConnection.kt:35-52
Timestamp: 2025-09-05T12:47:49.016Z
Learning: JettyWebsocketConnection in ktor-server-jetty-jakarta implements Closeable interface (has close() method), so connection.use { } is valid Kotlin syntax and will compile correctly.
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: In Ktor test files, particularly in the ktor-client/ktor-client-core/jvm/test/ directory, test files follow the convention of not including explicit package declarations. This is consistent across test files like CachingCacheStorageTest.kt and should be maintained for consistency.
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: In Ktor test files, particularly in the ktor-client/ktor-client-core/jvm/test/ directory, test files follow the convention of not including explicit package declarations. This is consistent across test files like CachingCacheStorageTest.kt and should be maintained for consistency.
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Run relevant tests with './gradlew jvmTest' to verify functionality before submission.
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Start with JVM-only implementation and tests unless the user specifically requests otherwise
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Start with JVM-only implementation and tests unless the user specifically requests otherwise.
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420
Timestamp: 2025-11-27T10:07:26.686Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.
Learnt from: zibet27
Repo: ktorio/ktor PR: 5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/build.gradle.kts:12-12
Timestamp: 2025-08-14T15:17:11.466Z
Learning: The Cargo plugin (dev.gobley.cargo) used in the Ktor WebRTC RS module depends on the kotlin("plugin.atomicfu") plugin, so atomicfu should not be removed even if there are no direct kotlinx.atomicfu imports in the module's source code.
Subsystem
WebRTC Client.
Motivation
KTOR-9188 WebRTC.
IceServer.urlsshould be a list.Solution
Change the type of
urls. I think it's ok, because the API is still experimental. Moreover,IceServersare usually initialised only once.I wanted to add more tests, ensuring ice servers are actually considered, but it doesn't work on emulators/simulators.