Rework how players produce warnings and errors. #670
Conversation
defunctzombie
force-pushed
the
roman/player-error-notifications
branch
from
4dfbdcb
to
49e1ba3
Compare
| @@ -66,62 +68,43 @@ type UserNodeActions = { | |||
| setUserNodeRosLib: SetUserNodeRosLib; | |||
| }; | |||
|
|
|||
| const rpcFromNewSharedWorker = (worker: SharedWorker, name: string) => { | |||
There was a problem hiding this comment.
I dislike the disconnect from call site and error hookup - regardless of that stylistic preference - the error handling needs access to member field now so I found it easier to put inline since some of the workers are userspace node specific
There was a problem hiding this comment.
It seems like more error handling code is duplicated now; why did this need to be removed?
There was a problem hiding this comment.
Error handling is different for the transform worker and the runtime worker. The runtime worker errors bucket by the nodeId so that we can surface errors from multiple userspace nodes and clear errors from individual userspace nodes. These errors happen when the worker itself may be running and we clear them if we get a successful message from the node. The error handling for the transform worker errors are not cleared and not tied to a specific node.
|
I am getting this (related?) error on current main branch. To produce, simply run
|
defunctzombie
force-pushed
the
roman/player-error-notifications
branch
3 times, most recently
from
837e413
to
1c8928e
Compare
defunctzombie
changed the title
|
@amacneil I am not able to reproduce with your instructions on this branch. |
| @@ -66,62 +68,43 @@ type UserNodeActions = { | |||
| setUserNodeRosLib: SetUserNodeRosLib; | |||
| }; | |||
|
|
|||
| const rpcFromNewSharedWorker = (worker: SharedWorker, name: string) => { | |||
There was a problem hiding this comment.
It seems like more error handling code is duplicated now; why did this need to be removed?
app/players/UserNodePlayer/nodeTransformerWorker/transformer.ts
Outdated
Show resolved
Hide resolved
Previously, players used sendNotification whenever they encountered a warning or an error. sendNotification is a one-way out-of-band reporting mechanism. Once the player produced the error it would persist in the app (and in the user's app bar) for the remainder of the app lifetime. This led to a confusing user experience when a player had a transient error that was resolved (i.e. reconnection after failed connection). This change removes sendNotification from all players and replaces it with a _problems_ entry in the player state. The player state is how players communicate their updates to the outside world. Problems are another form of player updates. By sending problems via the player state, a player can add/remove/clear problems as necessary. This change updates the player presence indicator ui to indicate the presence of problems in addition to the player presence. The user can see the list of problems by clicking the presence indicator and and further seeing any details a problem may expose.
defunctzombie
force-pushed
the
roman/player-error-notifications
branch
from
1c8928e
to
9716c74
Compare
Previously, players used sendNotification whenever they encountered a
warning or an error. sendNotification is a one-way out-of-band reporting
mechanism. Once the player produced the error it would persist in the app
(and in the user's app bar) for the remainder of the app lifetime. This
led to a confusing user experience when a player had a transient error
that was resolved (i.e. reconnection after failed connection).
This change removes sendNotification from all players and replaces it
with a problems entry in the player state. The player state is how
players communicate their updates to the outside world. Problems are another
form of player updates. By sending problems via the player state, a
player can add/remove/clear problems as necessary.
This change updates the player presence indicator ui to indicate the presence
of problems in addition to the player presence. The user can see the list
of problems by clicking the presence indicator and and further seeing any
details a problem may expose.
Here is an example of a player loading and indicating errors:
