More consistent error reporting

[cinnamon-bun]

What's the problem you want solved?

This code uses a variety of paradigms for reporting errors. Variously:

• Return null on error, with no access to the reason for error
• Raise exception
• Return { result, err }

Is there a solution you'd like to recommend?

Choose and standardize on one style.

Here are some options:
https://github.com/earthstar-project/earthstar/blob/master/ignore/errorstyles.ts

Criteria:

• Typescript understands it
• Usable from plain Javascript
• Easy to use (short code)
• Easy to understand (widely used)
• Returns rich error information
• Serializable (so errors can be returned from pub servers over HTTP)
• Works well with async functions

[cinnamon-bun]

Which of these would you most enjoy using? I just rewrote a bunch of code in style D (classic try/catch) and it's feeling messy now because Typescript can't track which functions throw exceptions and which don't.

Thoughts?

Actual working code examples are in errorstyles.ts.

A. Custom error type:

type Err = {err: string};
let makeDog = () : Dog | Err => {
    return {err: 'nope'};
}
let dog = makeDog()
if ('err' in dog) { console.log('oops'); }

B. Result style using the built-in node Error type.
(remember that Result<T> = T | Error)

let makeDog = () : Result<Dog> => {
    return new Error('nope');
}
let dog = makeDog()
if (dog instanceof Error) { console.log('oops'); }

C. Node style: [err, result]

let makeDog = () : [string | null, Dog | null] => {
    return ['nope', null];
}
let [err, dog] = makeDog()
if (err) { console.log('oops'); }
else { console.log(dog.name); } // dog could still be null here. :(
// Typescript can't enforce that exactly one of the array items is null and one is not

D. Classic throw/catch

let makeDog = () : Dog => {
    throw new Error('nope');
}
let dog = makeDog()
// oops, we forgot to catch that error because Typescript doesn't track it for us
// let's try again
try {
    let dog = makeDog()
} catch (err) {
    console.log('oops');
}

[sgwilym]

I like A! It allows you to build much richer error types, and TS can keep tabs on it. It's more work than B, but B shifts work onto clients which then have to parse strings from Errors into different kinds of errors.

As you mentioned, TS doesn't deal with throw/catch well, and is more unwieldy with async code.

[cinnamon-bun]

Done! It was a big commit.

Docs about the new error handling.

I chose style B but returning custom subclasses of the built-in node Error class. We can add custom fields, and easily check which kind of error it is using err instanceof ValidationError.

I thought it would be a bit more normal to use node Errors instead of making our own ad-hoc Error type. It also means we can throw them if circumstances demand it, which happens in a few places (especially class constructors which can't return errors so we have to throw them).

This is a breaking change. Things that used to return true/false on success/failure now return something different, so you can't just check for trueyness or falseyness:

// don't do this anymore
if (storage.set(...)) { // success! }

// do this
if (notErr(storage.set(...))) { // success! }

The new isErr and notErr helper functions check if something is an error.