Throw Error objects instead of strings

[feross]

The problem

I noticed that there are several places where you throw a string. It's considered best practice to only throw Error objects. When listening to an ondata event, it's really unexpected to have a variable called err that is a string instead of an Error object.

[101arrowz]

I originally did this to make the core algorithms as "low-level" as possible. Now I think that old logic is flawed. However, I've also considered using an enum of errors to keep errors more customizable by the consumer. Would that be acceptable as well?

[feross]

@101arrowz Can you explain what you mean by "using an enum of errors"?

The best way to do errors in my experience is to:

• Always throw Error objects

• Add a code property onto the Error object so that users can detect specific error types:

  if (err && err.code === 'INVALID_ZIP_CONTAINER') { /* ... Just an example ... */ }

Alternatively, I've seen some projects create many different subclasses of the Error object, for example InvalidZipContainer, etc. which the user can detect with err instanceof InvalidZipContainer. For an example of this, see https://vincit.github.io/objection.js/recipes/error-handling.html

I personally like the first approach since it's lighter weight and doesn't require the user to import a bunch more symbols into the project.

[101arrowz]

In TypeScript, there is an enum type that compiles to pure numbers. So it could be done like in lower level languages:

enum InflateError {
  InvalidDistance,
  InvalidLength,
  UnexpectedEOF
}

try {
  inflateSync(something);
} catch (e) {
  if (e == InflateError.InvalidDistance) {
    // handle
  }
}

On reconsideration maybe it's best just to use the system you suggested. I'll look into implementing this for v0.7.0.

[feross]

I see. Using enums for the code property on the Error object could work, but I wouldn't throw a bare number – it's really surprising behavior for a JavaScript library.

[nettybun]

I think an enum is great for bundle size. The library focuses on being small and adding very long error strings (or object keys) will be costly. Consumers can have the numbers translated back into their enum names by typings support which makes it less surprising.

[101arrowz]

@feross After more consideration I've come to the conclusion that since fflate is designed first and foremost as a lower-level library, I don't think having raw errors is an issue. The "high-level wrappers" are actually all necessary in order to achieve the functionality they provide; in other words, you can't actually create AsyncInflate without access to library internals. This is the same reason I never verify argument types, and the same reason I use callbacks over Promise for async functions.

If you wish, I can develop a higher-level wrapper for fflate with niceties such as argument validation, async/await for async compression, compatibility with WHATWG streaming spec for the streaming APIs, and better errors. However, for now I will only create an enum-based error system, since it makes it possible to create better errors to re-throw, rather than having to parse a string.

[101arrowz]

I've made progress with the enum-based error system but I'm reluctant to publish it because it makes life much more difficult when debugging as an end-user; at least the strings provided the actual error that occurred. I'm still considering the Error object system as a result, but if I do implement it, it will be very lightweight.

[101arrowz]

I've managed to implement errors with codes in a very lightweight fashion; it's even smaller than before. I will publish this in v0.7.0, hopefully in a few days.

[feross]

@101arrowz Sounds good. Thanks for your attention to detail. What approach did you go with?

[101arrowz]

An enum for the codes, but an error object is always thrown as you suggested. I'm still making final adjustments; since the package has become quite popular now I have to test each release more.

[101arrowz]

v0.7.0 is released; now there are nice errors with codes throughout the library. Let me know if you have any further issues.