search

repeaterjs / repeater

The missing constructor for creating safe async iterators
https://repeater.js.org
MIT License
459 stars 12 forks source link

Custom errors #27

Closed elderapo closed 5 years ago

brainkim commented 5 years ago

23 relevant issue

brainkim commented 5 years ago

I took a look at ts-custom-error and it is a 100-line dependency spread across 5 files which I’ve already managed to fully replicate in my copy-pasted error subclasses. It isn’t even painless in that you still need to add a name property to error classes, so that they don’t get clobbered when the code is minified.

Additionally, this PR creates error subclasses for errors which are not exposed in any meaningful way. I don’t really see the value of creating custom errors for buffer initialization errors (I think reusing RangeError is more idiomatic), nor do I see the use-case (yet) for catching Buffer empty/full errors. Also, I don’t think having custom message initialization logic in Error subclasses is a good idea, insofar as the messages should vary on a case-to-case basis and having them inline can clarify the intent of the error in the code.

I know this sounds silly, but I’m against merging this PR because it does not provide any additional functionality while adding a dependency to two of the packages in this repository. I know you said it was error prone, but I am happy to copy-paste my Error subclassing code until tc39 gets its shit together and creates a global custom error class.