Cast string into error while handling errors

[Exelord]

Summary

I am proposing to cast strings into errors, to make sure castError will always return the error. This will unify the behaviur and simplify errors handling by users. Also the original error, whether string or object, will be now attached as cause of the error.

How did you test this change?

pnpm test

[changeset-bot]

⚠️ No Changeset found

Latest commit: f40ca6a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[ryansolid]

This is good change probably. I did the casting mostly for undefined/null cases. I'm not sure it is expected if you throw a string to not get a string back but this does make things more homogenous. In any case this will require a more significant release than a patch to merge it.

[coveralls]

Pull Request Test Coverage Report for Build 4107676088

• 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 88.251%

---

Totals
Change from base Build 4106927886:	0.0%
Covered Lines:	1296
Relevant Lines:	1393

---

💛 - Coveralls

[Exelord]

Throwing anything other than Error is considered an anti-pattern as it does not contain a stack trace (not possible to track it down).

I agree that this change may cause breaking changes, however I believe it will point out existing issues rather than create them.

But sure, let's leave it for bigger release :)

[Exelord]

I have also strongly typed onError hook, as we will be able to guarantee now the error.

[Exelord]

@ryansolid also a question. Maybe we should only cast error in case we are passing it to onError hooks?

function handleError(err: unknown): void {
  const fns = ERROR && lookup(Owner, ERROR);
  if (!fns) throw error;
  const error = castError(err);
  for (const f of fns) f(error);
}

and in case they are not handled, we should throw them they way they are? This way we can fully control its type, without breaking change, and without enforcing the cast.

[ryansolid]

I needed it for resources as well. That was actually where a lot of issues were happening. Especially how it affected SSR and communicating across the wire.

[ryansolid]

@ryansolid also a question. Maybe we should only cast error in case we are passing it to onError hooks?

function handleError(err: unknown): void {
  const fns = ERROR && lookup(Owner, ERROR);
  if (!fns) throw error;
  const error = castError(err);
  for (const f of fns) f(error);
}

and in case they are not handled, we should throw them they way they are? This way we can fully control its type, without breaking change, and without enforcing the cast.

Re-reading. This is a good idea.

[kapilpipaliya]

when ctx.measureText error is not properly handled:

  let canvasElement:HTMLCanvasElement = document.getElementById("myCanvas");
  let ctx = canvasElement.getContext("2d");
  ctx.font = args.font;
  ctx.measureText(args.text).width // when this line throw error I am getting castError