String? where String is expected error

[fgndev]

Building Redmoon recent tools / compileSdkVersion 30 I ran into this bug. I just fixed it syntactically, as far as I can tell my solution works but is not gonna be helpful in the logs. I'm new to Android though so that's the best I can come up with right now.

[smichel17]

Hi, thanks! I'll explain a little, then you can decide whether you'd like me to merge as-is (it doesn't make anything worse!) or work on it a little more first.

The component of Red Moon that draws the overlay is the FilterService. It runs in the background, regardless of whether other parts of the app are open; those other parts control the service using the Android Intent system. They launch an intent for the filter service, and Android will deliver it to the running filter service (or start up a new one, if there is not already a running instance).

Intents can carry data in "extras", which are simple key-value pairs, but only strings/bool/int… (serialized data). So to say which command, we can't send the Command object, only its string equivalent. Then, upon receipt, the service calls Command.getCommand to get the actual Command to perform whatever action.

Because intents are constructed at runtime, there's no way for the compiler to validate whether the correct string is stored as an extra, or even there at all, hence the String? type. So, in compileSdkVersion 29, where the extra is typed as a regular String for some reason, the Command.values() call just below will throw if it was passed null. However, it will also throw if passed an invalid string (does not correspond to one of the enum values), so if the intent is not present, it will crash in the same place, regardless.

Red Moon guards against accidentally forgetting to add the extra by only generating the intent in one place, just a few lines above:

    val intent: Intent
        get() = intent(FilterService::class).putExtra(EXTRA_COMMAND, name)

Everywhere else just calls Command.ON.send() (or the same for whichever other command). So, I don't expect we'll ever run into this problem, and it would be just as fine to throw a !! on the end of getStringExtra() — either way Red Moon will crash with an uncaught exception, and hopefully whoever experienced it can grab a logcat to find out why it happened.

An alternative would be to something like this:

fun getCommand(intent: Intent): Command? {
    val commandName = intent.getStringExtra(EXTRA_COMMAND)
    Log.i("Recieved flag: $commandName")
    return commandName?.let {
        try {
            valueOf(it)
        } catch (e: IllegalArgumentException) {
            null
        }
    }
}

And then, up in the filter service, ignore it if the command is null. This way invalid intents are ignored, rather than causing a crash.

Personally I learn toward allowing it to crash, since I'd rather make it more obvious that something is wrong, so it can be fixed, rather than having silent errors. But I think both ways are reasonable.

[fgndev]

Hi, Thanks for the reply and the explanation. If I understood you correctly, my fix makes sure that an exception is thrown in case of an invalid intent which is clearly what I prefer to the invalid intent being silently ignored, no matter how unlikely that case may be. So I'd just leave the code as is.