Skip to content

ShareModal discards the host's visibility-change error text (double .error hop on a string) #1789

Description

@simihablo

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)

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions