Conversation
mhoyer-streamlabs
changed the title
| usesStream = false; | ||
| auto response = conn->call_synchronous_helper(className, "SetRecording", {ipc::value(this->uid), ipc::value(UINT64_MAX)}); | ||
| ValidateResponse(info, response); | ||
| usesStream = true; |
There was a problem hiding this comment.
Why did you change usesStream = true; ? This changes the behavior entirely. Likely a copypaste issue.
There was a problem hiding this comment.
Definitely looks like copy/paste. Although should we even set this here? If setting the new recording object fails and we're going to leave the previous object, shouldn't we leave the previous value of this var as well?
| usesStream = false; | ||
| auto response = conn->call_synchronous_helper(className, "SetRecording", {ipc::value(this->uid), ipc::value(UINT64_MAX)}); | ||
| ValidateResponse(info, response); | ||
| usesStream = true; |
There was a problem hiding this comment.
usesStream = true;
The same here.
|
|
||
| void osn::Streaming::SetVideoEncoder(const Napi::CallbackInfo &info, const Napi::Value &value) | ||
| { | ||
| if (!videoEncoderRef.IsEmpty()) |
There was a problem hiding this comment.
Resetting the cached ref before the IPC call succeeds can leave the wrapper out of sync with the backend. If SetVideoEncoder rejects the new encoder, the backend keeps the old encoder but videoEncoderRef has already been cleared, so the client starts returning undefined. Can we move the reset until after ValidateResponse succeeds?
This same pattern also appears in other places.
There was a problem hiding this comment.
You are correct - I was thinking a backend failure meant no encoder stored, but I see now that it actually keeps any previous value in both client and server. My thought was not to store the previous value in an error case, but I only updated half of that. I'll move those statements back down so the previous value remains.
3 participants
Description
Update client error handling to ensure all errors are passed along to the front end by replacing
conn->callwithconn->call_synchronous_helperand parsing the return. In some streaming/recording/replay buffer cases, clear the existing object ref and don't set the new one if the back end call fails. Replacedstd::vector<ipc::value>withautoto shorten calls and make them easier to read.Motivation and Context
Some important errors were being ignored.
How Has This Been Tested?
Types of changes
Checklist: