ShareModal discards the host's visibility-change error text (double .error hop on a string)
@MAYANKSHARMA01010 is already working on this.
Since Aug 24, 2026.
- Dominant language
- TypeScript
- Stars
- 137
- Forks
- 239
- Avg merge
- 4d 14h
- Merged PRs (30d)
- 5
Description
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/sistent0.22.0 (verified against the published npm artifact, not a local build)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.