cancel
Showing results for 
Search instead for 
Did you mean: 

[Bug] Platform - Creating Invalid Requests causes CallbackRunner to silently consume Notifications

Aurelinator
Explorer
Unity 2018.4.3f1 | Oculus Unity Integration 1.38

Hi there,
I'm just posting this to identify a bug I spent a while trying to diagnose that ate up my past week. I'm hoping that if anyone else has this issue, this might help them, and that proper steps may be taken to resolve this for newcomers using the Unity Integration tools for the first time. Here's the issue:

When making a request that is "invalid", the Unity integration sometimes returns a proper request, but it has Request ID of 0. Any time that this happens, if I register a callback for it, Platform Notifications will be eaten, which may cause random, problematic bugs. Why does this happen?

Oculus's Callback.cs contains two separate dictionaries mapping callbacks. In one, you have callbacks for regular repeating things - Room Data Updates, Invite Notifications, Voip Connectivity Changes, etc. In the other, you have mappings of Request IDs to callbacks, where Request IDs are sequential integers that are incremented. When a message is popped off of the ovr message stack, it goes into Callback.cs - HandleMessage. If there's a callback in the `requestIDsToCallbacks` dictionary, it uses it. Otherwise, it tries to see if it was registered in the notification dictionary.

The problem is, if you EVER register a callback for a Request with ID 0, Notifications will be eaten up and error. This is because all notifications have Request ID = 0, which means that, TECHNICALLY, there is a callback waiting for Request 0, so that notification will disappear.

------------------------------------------------------

Here's an example. I have a project with a bunch of social properties around Rooms. I want to change the Room datastore to reflect certain environment changes. I can use the UpdateDataStore/ovr_Room_UpdateDataStore call to update my stuff. Well, it turns out that UpdateDataStore returns a Request, and I can schedule a callback for it, to know if the call succeeded or not. So let's say I have part of a call where I want to add new key/value pair to the room. Here's how the final call would look.

...
data[key] = value;
Rooms.UpdateDataStore(Room.ID, data).OnSuccess("Rooms.UpdateDataStore", (room) =>
{
Debug.Log($"[Room] Successfully updated datastore on room ({room.ID}) with [{key}] | [{value}]");
});

^ in the code above, OnSuccess is just an extension method that wraps around the default OnComplete:
public static void OnSuccess<T>(this Request<T> request, string actionName = null, Action<T> callback)
{
request.OnComplete((message) =>
{
if(message.IsError)
{
if(string.IsNullOrEmpty(actionName))
actionName = "Unknown Oculus Request";


var error = message.GetError();
error.Log(actionName); // uses my own extension method for logging prettily

return;
}
else
{
callback(message.Data);
}
});
}

Either way, that's not important. But imagine that I have this nice wrapper around OnComplete that logs in case of failure, and calls my callback in case of a success. This works most of the time. But... let's say that I am in a Solo Room. In that case, calling UpdateDataStore works just fine. It returns a Request, with Request ID 0. My code has been working just fine, and I schedule my callback.

But... apparently, it was an error. What's the point of updating the datastore in a solo room if there's nobody to hear it? And because that request was never meant to happen, it doesn't return a proper Request that will return an Error in the Message. It just... is scheduled as Request ID 0, and it will never, ever, ever pop off the stack. It just sits there.

Now, when a notification comes in - any notification at all, they all technically have Request ID 0. They will attempt to attempt to fire the callback for my "successfully updated the datastore" but it will error out during the cast, and log an error on line 142 of Callback.cs (Debug.LogError("Unable to handle message: ")... as

-------------------------
Repro:

1. Schedule a notification callback for some regular repeating notification.
2. Call UpdateDataStore in a solo room and schedule the OnComplete callback.
3. The callback will never fire.
4. When the next Notification that comes in, it will try to fire that callback and error out, meaning that your notification callback will not get called.

--------------------------

I use UpdateDataStore here as an example, but I've seen it happen in a number of other cases, for example, if you call Rooms.Get(currentRoomId).OnComplete(...), where your currentRoomId is 0 (solo room), it will return a proper Request with RequestID = 0 , that will never get called, and trigger this. I had some logic that refreshed the current room in certain conditions, and noticed that it was causing the same issue. Clearly, I should've used GetCurrent, but the point still stands, that I would have rather gotten an error in the callback, rather than a silent no-op that never triggers, and causes me to have intermittent broken failures.

The fix on the Oculus side should be pretty easy. You guys need to either:
a) Handle the case where the Request ID is 0 and not schedule it.
b) Process notifications before processing requests in HandleMessage
c) Not return requests with id = 0, but schedule proper requests with an error so that people can know what they are doing wrong.
d) Change Notifications to have RequestId = -1 (which won't work since you're using ulongs for the request ID).
e) Force all users to check every request that ever triggers to see if it is 0, and not actually schedule the callback in that case. (What I did on my end using an extension method).

TLDR:
As it currently stands, any time the Oculus Platform API calls returns a Request/Request<T> with an ID of 0, the next notification message will be swallowed up and not fired in the application which will cause indeterminate errors. I've handled it on my end, but this might cause tons of problems for other people running into intermittent notification disappearances.
0 REPLIES 0