Summary
ShareModal's visibility-change notification always renders as a bare "Failed to update visibility. " with no detail, because it dereferences .error twice on a value its own prop type declares as a plain string.
Where
ShareModal, in the published @sistent/sistent@0.22.0 bundle (dist/index.mjs):
const B = `Failed to update visibility. ${q?.error?.error || ""}`;
q.error
? d({ message: B, event_type: "error" })
: d({ message: N, event_type: "success" });
q here is the awaited result of the host-supplied handleUpdateVisibility prop.
Why it is wrong
ShareModalProps types that prop as:
handleUpdateVisibility: (value: string) => Promise<{ error: string }>;
So q.error is a string. The branch test q.error is correct, but the message interpolates q.error.error - a .error lookup on a string - which is always undefined, so || "" collapses it to the empty string. Every message a host composes is discarded and the user is told only that something failed, never what.
The adjacent revoke path is fine, since there the value genuinely is an RTK result:
{ error: N?.error?.error }
That is likely where the extra hop was copied from.
Impact
Hosts have no way to surface a reason for a failed visibility change. In Kanvas (layer5labs/meshery-extensions) we normalize RTK's several error shapes into the documented { error: string } and return a specific message for unsupported resource kinds; none of it can reach the user.
This is in the same silent-failure family as the payload casing bug fixed in #1786 - the failure is now correctly detected (a 4xx/5xx no longer reports as success), but the explanation is dropped on the floor.
Suggested fix
const B = `Failed to update visibility. ${q?.error || ""}`;
If the double hop is there to tolerate hosts that return an RTK result rather than the documented shape, normalizing once would be clearer than an optional-chained guess:
const detail = typeof q?.error === "string" ? q.error : (q?.error?.error ?? "");
Reproduce
Pass a handleUpdateVisibility that resolves to { error: "visibility rejected" } and trigger a visibility change. Expected: the reason appears in the error notification. Actual: "Failed to update visibility. ".
Environment
@sistent/sistent 0.22.0 (verified against the published npm artifact, not a local build)
Summary
ShareModal's visibility-change notification always renders as a bare"Failed to update visibility. "with no detail, because it dereferences.errortwice on a value its own prop type declares as a plain string.Where
ShareModal, in the published@sistent/sistent@0.22.0bundle (dist/index.mjs):qhere is the awaited result of the host-suppliedhandleUpdateVisibilityprop.Why it is wrong
ShareModalPropstypes that prop as:So
q.erroris a string. The branch testq.erroris correct, but the message interpolatesq.error.error- a.errorlookup on a string - which is alwaysundefined, so|| ""collapses it to the empty string. Every message a host composes is discarded and the user is told only that something failed, never what.The adjacent revoke path is fine, since there the value genuinely is an RTK result:
That is likely where the extra hop was copied from.
Impact
Hosts have no way to surface a reason for a failed visibility change. In Kanvas (layer5labs/meshery-extensions) we normalize RTK's several error shapes into the documented
{ error: string }and return a specific message for unsupported resource kinds; none of it can reach the user.This is in the same silent-failure family as the payload casing bug fixed in #1786 - the failure is now correctly detected (a 4xx/5xx no longer reports as success), but the explanation is dropped on the floor.
Suggested fix
If the double hop is there to tolerate hosts that return an RTK result rather than the documented shape, normalizing once would be clearer than an optional-chained guess:
Reproduce
Pass a
handleUpdateVisibilitythat resolves to{ error: "visibility rejected" }and trigger a visibility change. Expected: the reason appears in the error notification. Actual:"Failed to update visibility. ".Environment
@sistent/sistent0.22.0 (verified against the published npm artifact, not a local build)