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

Fix/improve errors #2394

Merged
merged 5 commits into from
Aug 4, 2022
Merged

Conversation

lukekugelman
Copy link
Contributor

Creating this PR to align error handling in RTF with Javascript best practices. I was running into a problem with another library where it was expecting any exceptions thrown to be errors (with the property cause).

So the intent is to replace anytime a string was thrown in the past with a FiberError as to better handle any exceptions that RTF does not handle itself.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 26, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 839542d:

Sandbox Source
example Configuration

@CodyJasonBennett
Copy link
Member

CodyJasonBennett commented Jul 26, 2022

I'm confused about the premise of this PR. Would it be sufficient to throw an Error here? Why abstract it into FiberError?

On a related note, I'm de-escalating text errors into warnings with #2395.

@lukekugelman
Copy link
Contributor Author

lukekugelman commented Jul 26, 2022

Using Error would suffice. FiberError was created to allow developers to differentiate and handle RTF specifically if desired (by checking instanceof).

This is still needed for the other errors that are thrown by RTF, apart from the text errors.

@CodyJasonBennett CodyJasonBennett merged commit a58116c into pmndrs:master Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants