Add onMissingImplementationError hook to RemoteReceiverOptions#603
Add onMissingImplementationError hook to RemoteReceiverOptions#603cpeddecord wants to merge 1 commit intomainfrom
Conversation
|
We detected some changes in public packages, and there are no updates in the |
When connection.call() is invoked on a node with no registered implementation, the hook is called with a context object containing the node id, method name, args, and element type. The consumer can return a value, throw a custom error, or return THROW_DEFAULT to fall through to the default error. This replaces the need to monkey-patch connection.call and duck-type error messages when customizing error handling for missing remote method implementations. Co-authored-by: Pi Agent <pi-agent@noreply.com>
cpeddecord
force-pushed
the
clp/on-missing-implementation-hook
branch
from
b923998 to
892890a
Compare
kumar303
left a comment
kumar303
left a comment
There was a problem hiding this comment.
I'm struggling to imagine how this would be used. Is it just to observe and log the error? Is it so the host can render a no-op element? Is it to work around what feels like a bug in how elements are registered?
| export const THROW_DEFAULT: unique symbol = Symbol.for( | ||
| 'remote-dom.throw-default', | ||
| ); |
There was a problem hiding this comment.
Throwing is the default in this case but a default is a default, right?
| export const THROW_DEFAULT: unique symbol = Symbol.for( | |
| 'remote-dom.throw-default', | |
| ); | |
| export const DEFAULT: unique symbol = Symbol.for( | |
| 'remote-dom.default', | |
| ); |
There was a problem hiding this comment.
This change doesn't seem to have any test coverage. I guess that's because there's no suite for this at all?
There was a problem hiding this comment.
I can backfill some tests here for sure.
|
@kumar303 essentially that, yes. Right now a consumer would have to monkeypatch their own SignalRemoteReceiver implementation to inspect the the error and then duck type the emitted error to see if it matches. |
|
@cpeddecord if it's observational only then why not just throw a more specific error that can be caught? throw new NodeNotImplementedError(
`Node ${id} does not implement the ${method}() method`,
// Maybe some metadata if you need it?
metadata: {...}
); |
|
@kumar303, could do, that and all other errors. |
2 participants
tl;dr
when a node either mounts before or after a host is ready or has deleted a components remote method, we can sometimes be left with an invalid remote method call. This PR adds a hook for receivers to capture errors related to that error.
Why
If we have isolated the error to faulty third-party code we may want to do something other than throw an error. This PR adds an optional hook that can accept the error or return a sentinel to fall back to the receiver's default thrown error.