OnWsClose を自分自身の時にも発火するようにする #133
Merged
Conversation
This file contains 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
|
@melpon 切断しようとしたのにエラーが出て切断できなかった時には発火しないように周囲の実装を見て判断して実装したのですが、この実装でカバーできているのかはお教えいただけると助かります。 |
melpon
reviewed
Sep 29, 2024
src/sora_signaling.cpp
Outdated
| config_.websocket_close_timeout); | ||
| ws_->Close( | ||
| [self = shared_from_this()](boost::system::error_code ec) { | ||
| if (!ec) { |
There was a problem hiding this comment.
Close でエラーが起きてもイベントを投げたほうがいいんじゃないかって気もしますけどどうなんだろう…。
There was a problem hiding this comment.
たしかに。でも、その場合はエラーが起きてることを通知に含めるのも必要かなと思いつつ 。。。
@voluntas internal_error = 1011 でも code に入れて出すことにします？別で Python SDK からログは多分出るので。reason は SELF-CLOSED のままで。
There was a problem hiding this comment.
エラーは 4999 入れてもらえれば。ノーマルなエラーはできるだけ使いたくないので。SDK 側のエラーは全部 4999 にしてます。
There was a problem hiding this comment.
Sora から以上を通知 -> 4490
クライアントから以上を通知 -> 4999
くらいでおｋです。これ on_ws_close だけでなく on_disconnect でもとれるようにしたいと思ってます。（大改革中）
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
OnWsClose を自分で閉じた際にも切断時に発火するように変更します。
close_code::normal (1000) で発火させるようにしました。切断したのか切断されたのかわかった方がいいんじゃないかと思わなくもないのですが。。。