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

Assignees

No one assigned

    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