Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Replace getErrorMessage with {cause} or better message extractor #7218

Closed
fregante opened this issue Dec 28, 2023 · 4 comments
Closed

Comments

@fregante
Copy link
Collaborator

I think getErrorMessage should be used sparingly, we should review most usage in the extension. For example:

  • Extracting the message to create a new Error
    This loses the original stack and possible nested causes. Solution: Use {cause}
    throw new Error(
    `Control Room rejected login. Verify you are a user in the Control Room, and/or verify the Control Room SAML and AuthConfig App configuration.
    Error: ${getErrorMessage(error)}`,
    );
  • Showing the error to the user
    • We can also show the cause, which can add more information (Error saving spreadsheet. \n Google API request failed. \n No internet connection)
      • We already have getErrorMessageWithCauses
      • The notification code is already set up for this, showing details on successive lines
    • Have a single React component take care of displaying an Error instance/object instead of passing strings around. This can potentially also include the stack on DEV builds
      • <ErrorDetails error={error} stack/>
      • <ErrorDetails error={error} showDetailsButton/>
      • There are actually already several of these components, each doing things a bit differently
  • Changing the error type
    throw new RemoteServiceError(getErrorMessage(error), { cause: error });
  • Wrapping errors with inline error objects
    error: {
    name: "BlockRegistryError",
    message: `Error loading brick from registry\n${getErrorMessage(
    blockError,
    )}`,
    },

Related

Previous work on this

@fregante
Copy link
Collaborator Author

Here's another use case for a standardized error displayed component: automatic linkification of URLs.

Screenshot 8

Seen in #4775

@fregante fregante added the rfc label Jan 27, 2024
@fregante
Copy link
Collaborator Author

fregante commented Jan 29, 2024

Also often error states are not tested so they end up without proper padding. There are at least 3 ways to show errors in the sidebar:

Screenshot 4
Screenshot
Screenshot 1

Seen in #7469 and #7514

Copy link

This issue will be closed in 7 days unless the stale label is removed, or a comment is added to the issue.

@github-actions github-actions bot added the Stale label May 19, 2024
Copy link

This issue was closed because it has been stale for 7 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant