Skip to content
This repository has been archived by the owner on Nov 25, 2021. It is now read-only.

fix: use error names, not error codes #240

Merged
merged 1 commit into from
Apr 2, 2020
Merged

fix: use error names, not error codes #240

merged 1 commit into from
Apr 2, 2020

Conversation

felixfbecker
Copy link
Contributor

@felixfbecker felixfbecker commented Apr 1, 2020

Errors in ECMAScript have a message and a name. code is something Node-specific, and those errors still all have name. Native errors like TypeError, URIError, AggregateError (coming soon) all only have name. When we send errors to/from background pages or web workers, we can only reasonably expect name to be present, so we should never rely on code. See e.g. mozilla/webextension-polyfill#210 or comlink source code.

Using both in our codebase inconsistently opens the risk to checking for the wrong property, (especially because errors are any in TypeScript) and adds additional code.

This change removes all uses of code in favor of name. There is still a case for having name be optional (e.g. GraphQL errors don't have a name, but do have message), so ErrorLike is still kept for now but we could think about just using the native Error interface in the future.

@felixfbecker felixfbecker requested a review from a team April 1, 2020 21:19
@codecov
Copy link

codecov bot commented Apr 1, 2020

Codecov Report

Merging #240 into master will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #240   +/-   ##
=======================================
  Coverage   84.98%   84.98%           
=======================================
  Files          13       13           
  Lines         566      566           
  Branches      141      141           
=======================================
  Hits          481      481           
  Misses         85       85           
Impacted Files Coverage Δ
src/errors.ts 37.50% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af2eb47...046e6fc. Read the comment docs.

Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow what an assertion 😬

@felixfbecker felixfbecker merged commit d4ae210 into master Apr 2, 2020
@felixfbecker felixfbecker deleted the error-name branch April 2, 2020 17:30
@sourcegraph-bot
Copy link

🎉 This PR is included in version 6.3.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants