Skip to content

Conversation

@zztkm
Copy link
Contributor

@zztkm zztkm commented Apr 1, 2025

  • [FIX] Sora から切断された場合の切断処理を修正し適切なエラーを MediaChannelHandlers.onDisconnect で受け取ることができるようにする
    • Sora iOS SDK 2025.1.1 までは Sora から Close Frame を受け取ったり、ネットワークエラーが起きたりしても、WebSocket メッセージ受信失敗に起因する SoraError.webSocketError しか受信できなかったが、以下の内容を受信できるようになった
      • Sora から Close Frame を受け取った場合のステータスコードと理由
        • ステータスコードが 1000 で正常に切断された場合も MediaChannelHandlers.onDisconnect で通知する
      • ネットワークエラーや Sora がダウンした場合のエラー内容
    • この変更によって MediaChannelHandlers.onDisconnect で受信するメッセージの内容は変わるが、コールバックが発火するタイミングに変更はないうエラーしか返せていなかった状態が改善された

This pull request includes several changes to improve WebSocket handling and error reporting in the Sora project. The most important changes focus on enhancing the disconnect process and improving logging for WebSocket operations.

Improvements to WebSocket handling and error reporting:

Documentation updates:

  • CHANGES.md: Updated the change log to reflect the improvements in WebSocket disconnection handling and the addition of new encoding parameters for simulcast video.

Code comments:

  • Sora/SignalingChannel.swift: Updated comments to clarify the behavior of the onDisconnectWithError handler for WebSocket disconnections.

@zztkm zztkm self-assigned this Apr 1, 2025
CHANGES.md Outdated
Comment on lines 26 to 30
- [UPDATE] Sora から切断された場合の切断処理を改善する
- Sora から Close Frame を受けとった場合にステータスコードと理由を `MediaChannelHandlers.onDisconnect` で返すようになった
- ネットワークエラーや Sora がダウンした場合のエラー内容を `MediaChannelHandlers.onDisconnect` で返すようになった
- いままでは WebSocket メッセージ受信失敗時のエラーである `The operation couldn’t be completed. Socket is not connected` というエラーしか返せていなかった状態が改善された
- @zztkm
Copy link
Contributor Author

Choose a reason for hiding this comment

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

今回の変更内容はここです。

@zztkm zztkm requested a review from Copilot April 2, 2025 10:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aims to improve the disconnection behavior when Sora disconnects and update the changelog accordingly.

  • Updated the WebRTC version.
  • Improved the disconnection handling for Sora in the changelog, including returning status codes and reasons via MediaChannelHandlers.onDisconnect.
  • Added the encoding parameter "scaleResolutionDownTo" for simulcast video.
Files not reviewed (2)
  • Sora/SignalingChannel.swift: Language not supported
  • Sora/URLSessionWebSocketChannel.swift: Language not supported

@zztkm zztkm requested a review from miosakuma April 2, 2025 10:09
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@zztkm zztkm requested a review from Copilot April 2, 2025 10:12
@zztkm zztkm changed the title [WIP] Sora から切断された場合の挙動を修正する Sora から切断された場合の切断処理を改善する Apr 2, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request improves the handling of WebSocket disconnections and error reporting for Sora while updating the changelog to reflect these changes and added simulcast video encoding parameters.

  • Updated Sora disconnection processing to return error details via MediaChannelHandlers.onDisconnect
  • Enhanced logging for WebSocket send/receive errors
  • Documented the simulcast video encoding parameter change
Files not reviewed (2)
  • Sora/SignalingChannel.swift: Language not supported
  • Sora/URLSessionWebSocketChannel.swift: Language not supported

@zztkm zztkm requested a review from Copilot April 4, 2025 09:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

Sora/URLSessionWebSocketChannel.swift:166

  • [nitpick] The disconnect call on message reception failure has been removed in favor of handling it in didCompleteWithError. Consider adding a more detailed comment explaining this design decision for clarity.
case .failure(let error):

@zztkm zztkm requested a review from Copilot April 4, 2025 09:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

disconnect(error: error)
}
// 2025.2.x から、ステータスコード 1000 の場合でも error として上位層に伝搬させることにする (上位層が error 前提で組まれているためこのような方針にした)
// これは disconnect が error を渡さないと機能
Copy link
Contributor

Choose a reason for hiding this comment

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

文章が途中で切れていないでしょうか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

すみません修正漏れでした。そこの文章はいらないので削除しようと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

指摘ありがとうございます。

2b8d2de で修正しました

Copy link
Contributor

@miosakuma miosakuma left a comment

Choose a reason for hiding this comment

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

コメントが不備ではないかという指摘をしましたが、あとは大丈夫です。

@zztkm zztkm merged commit fcf335f into develop Apr 7, 2025
4 checks passed
@zztkm zztkm deleted the feature/fix-disconnect-process-from-sora branch April 7, 2025 03:35
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