dab-adapter-rs crashes on processing error messages

[yulian5]

Hi @arun-madhavan-013, Here is another problem:

If dab-adapter-rs receives excepted "success":true message then everything fine. However if command is not successful (when response is "success":false) then dab code crashes. Here is two examples:

RDK request: {"jsonrpc":"2.0","id":3,"method":"org.rdk.RDKShell.getVisibility","params":{"client":"youtube","callsign":"youtube"}}
RDK response: {"jsonrpc":"2.0","id":3,"result":{"message":"failed to get visibility","success":false}}
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("missing field `visible`", line: 1, column: 87)', src/device/rdk/applications/launch.rs:302:68
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Error channel tx: RecvError

and another:

RDK request: {"jsonrpc":"2.0","id":3,"method":"org.rdk.RDKShell.launch","params":{"callsign":"YouTube","type":"Cobalt","configuration":"{\"url\":\"https://www.youtube.com/tv?\"}"}}
RDK response: {"jsonrpc":"2.0","id":3,"result":{"message":"failed to launch application","success":false}}
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("missing field `launchType`", line: 1, column: 91)', src/device/rdk/applications/launch.rs:177:74
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Error channel tx: RecvError

Receiving error message is a normal situation and should be process accordingly without panic and crash.

[arun-madhavan-013]

Hi @yulian5 , As per Documentation; https://rdkcentral.github.io/rdkservices/#/api/RDKShellPlugin?id=result-30 and https://rdkcentral.github.io/rdkservices/#/api/RDKShellPlugin?id=result-40 says that these two result.visible in RDKShell.getVisibility and result.launchType in RDKShell.launch are not optional parameters unless there is an error condition. It would be wise to check why these APIs are returning failure response instead of handling the failure cases.

[yulian5]

Hi @arun-madhavan-013,

It would be wise to check why these APIs are returning failure response instead of handling the failure cases.

No it wouldn't. It would be much wiser to fix dab-adapter-rs adding processing error messages and as for that matter misformatted or bad responses and do not crash it during that. Pointing fingers was never a good practice especially in direction of quite mature RDKShell. RDKShell in that regard is much superior piece of code: it doesn't crash on bad commands and wrong messages as dab-adapter-rs does.

Follow your logic about documentation they should crash browser if user inputted the bad URL as there is no documentation how to render bad URL. Is that right?

It looks like RDKShell documentation https://rdkcentral.github.io/rdkservices/#/api/RDKShellPlugin was written by extremely optimistic and positive guys: it's only a few places where they mentioned about errors but everywhere else they just ignored them.

But if you check RDKShell code you can easily see that almost every command and request has response that include the error part:

for getVisibility:

        result = getVisibility(client, visible);
        if (false == result) {
          response["message"] = "failed to get visibility";
        }
        else {
          response["visible"] = visible;
        }

For setVisibilty there are multiple error cases. For launch and launchApplication there are also multiple responses including:

        if (!result) 
        {
            response["message"] = "failed to launch application";
        }

The dab-adapter-rs itself produces quite a number of error messages but you somehow denied that right for RDKShell. Error message itself doesn't mean that something wrong with the program which returns it. There are multiple legitimate reasons for that. And whether it might make sense to check this or not really depends on the error message and situation. But crashing on receiving error or bad message usually means just bad design.

Let check some reasons for launch command error message: the parameter of app name is incorrect, the app doesn't exist on the target, the app binary or dependency corrupted or missing, the app configuration is misconfigured, the system is out of memory, the disk is full, the app has wrong permissions, etc., etc.

The most of those reasons are applied to getVisibility as well plus the app may crash between its launch and request for the properties.

DAB protocol and its ideas suggest that it works remotely. And ability to check error condition sometimes doesn't exist. It's easy if it located on the same desk. But STB box or TV maybe located in another state or country in some user home. How you even know what was the error if you crash dab-adapter-rs and doesn't process/return error message?

It renders remote possibilities of dab-adapter-rs quite useless.

[arun-madhavan-013]

Hello @yulian5 ,

Sorry for the delay and I got your point.

Changed dab-adapter to accept the error message and map that to DAB error. We'll be able to run tests on which ever device we have access to where we never seen such failure.

Also; i've requested the documenttaion team to include the Error/Failure details also in the API documentation.

Feel free to contribute as this is a pure open source implementation.

[yulian5]

Hello @arun-madhavan-013,

Thanks for this work! I'm currently testing the changes. So far it looks good and dab-adapter-rs shows error and error message without crashes. I'll update with more info shortly.

[yulian5]

Hello @arun-madhavan-013,

Your changes look good! No crashes whatever I tried. Here is example report from error I've got without crash:

processing: system/settings/get RDK request: {"jsonrpc":"2.0","id":19,"method":"org.rdk.UserPreferences.1.getUILanguage"} RDK response: {"jsonrpc":"2.0","id":19,"error":{"code":2,"message":"Service is not active"}} Publishing response: dab/_response/system/settings/get {"status":500,"error":"Service is not active"}

Here are some simulated errors:

RDK response: {"jsonrpc":"2.0","id":3,"result":{"message":"failed to launch application. type not found","success":false}} Publishing response: dab/_response/applications/launch {"status":500,"error":"Error from org.rdk.RDKShell.launch failed to launch application. type not found"}

Ready to process DAB requests processing: applications/launch RDK request: {"jsonrpc":"2.0","id":1,"method":"org.rdk.RDKShell.getItsState"} RDK response: {"jsonrpc":"2.0","id":1,"error":{"code":-32601,"message":"Unknown method."}} Publishing response: dab/_response/applications/launch {"status":500,"error":"Unknown method."}

Thanks! Good job!