[APT-10372] Prevent Crashing If Clients Act Too Early #45
Conversation
The `ActionBeforeSetup` exception is being thrown from an area designed to handle errors, resulting in crashing. We catch these exceptions and update the error state instead.
kabliz
force-pushed
the
kabliz/APT-10372-action-before-ready
branch
from
7bce631 to
cad04f5
Compare
| else -> throw UnrecognizedAction(action) | ||
| else -> throw UnrecognizedAction(action) | ||
| } | ||
| } catch (ex: ActionBeforeSetup) { |
There was a problem hiding this comment.
Instead of this, can we handle ActionBeforeSetup in the media session? The whole idea behind this was throwing an error if it was called too soon, so it should be on the client to handle it.
There was a problem hiding this comment.
Imho moving it to the error field of ArmadilloState is the consistent, intended way errors are to be handled. Clients should be looking into the ArmadilloState for errors, and they shouldn't be expected to look in a 2nd place for them. That's not consistent, and ArmadilloState is a more graceful way to handle these errors without a crash risk.
There was a problem hiding this comment.
@kabliz LGTM, but wondering why we're not doing this for UnrecognizedAction too
| else -> throw UnrecognizedAction(action) | ||
| else -> throw UnrecognizedAction(action) | ||
| } | ||
| } catch (ex: ActionBeforeSetup) { |
There was a problem hiding this comment.
@kabliz LGTM, but wondering why we're not doing this for UnrecognizedAction too
The `ActionBeforeSetup` exception is being thrown from an area designed to handle errors, resulting in crashing. We catch these exceptions and update the error state instead.
The
ActionBeforeSetupexception is being thrown from an area designed to handle errors, resulting in crashing. We catch these exceptions and update the error state instead.