Question on ResultObject design

[headlessme]

Is there a reason to return a structured object that contains an error / value property instead of using the more standard resolved / rejected promise that works with normal JS error handling flows? Are the Promises you're returning guaranteed to never throw an error?

I'm finding it makes it easy to miss error handling as it not the standard try / catch usually seen with async functions.

The design seems to be forcing a lot of duplicate handling of errors, e.g.:

try {
  const result = await storage.addFileFromBuffer(...)
  if (result.error) handleErrors(result.error)
  else doSomethingElse(result.value)
} catch (err) {
  handleError(err)
}

[abudaan]

All errors get caught in the library code so it is not necessary to use a try/catch block in your code if you call a library method:

const {value, error} = await storage.addFileFromBuffer(...)
if (error !== null) {
    handleErrors(error)
} else {
    doSomethingElse(value)
}

The reasoning behind this approach is:

• you don't need to use try/catch for any library method
• it is possible to make error messages uniform across the various adapters
• all library methods always resolve, and resolve to the same type of object

[headlessme]

Thanks for the detail, and thanks for all your hard work on this library, it's much appreciated!

Just wanted to leave a few thoughts for consideration in case you decide to do a major bump again...

Patterns I've seen like this in the past can lead to usage such as:

const { value } = await storage.addFileFromBuffer(...)
...possibly do something with value...

It makes it pretty easy to accidentally ignore errors, especially if you rely on generic error bubbling / exception handling (e.g. as express middleware) as it's forcing developers outside the error control flow norms. Worst case it silently ignores errors, best case you probably end up hiding the actual error behind a null pointer exception or similar.

Again thanks for all your work on this, feel free to close this as it's just feedback that's come from our usage!

[abudaan]

Yes, I see your point. The current approach is a bit inspired by the Rust Result type. But I do agree that most java- and typescript programmers expect a more javascript-like exception handling.

The first thing that comes to my mind is to add a global switch to the library that can be set when you instantiate a Storage:

const s = new Storage({
   provider: "S3",
   throwExceptions: true,  
})

And then in the AbstractAdapter we fork all API methods, for instance fileExist:

// returns either a ResultObject or a boolean
public async fileExistsFork(
  ...args: [bucketName: string, fileName: string] | [fileName: string]
): Promise<ResultObjectBoolean | boolean> {
  const { bucketName, fileName, options: _o, error } = this.getFileAndBucketAndOptions(...args);
  if (error !== null) {
    return this.forkResult<string, ResultObject>({ value: null, error: error as string }) as ResultObjectBoolean;
  }

  const r = await this.checkBucket(bucketName);
  if (r.error !== null) {
    return this.forkResult<string, ResultObject>(r) as ResultObjectBoolean;
  }

  const r2 = await this._fileExists(r.value as string, fileName as string);
  return this.forkResult<boolean, ResultObjectBoolean>(r2)
}

protected forkResult<T1, T2 extends ResultObjectGeneric>(result: T2): T1 | T2 {
  if (this.config.throwErrors === true) {
    const { value, error } = result;
    if (error !== null) {
      throw new Error(error);
    } else {
      return value as T1;
    }
  } else {
    return result;
  }
}

[only-cliches]

Adding some of my own $0.02 here. I love Rust enough to have written a library to implement Rust style enums in Typescript, but it's my strong belief that Result style error handling should really be opt-in.

As much as I'd love to sprinkle my code with question marks and get Result types back all over the place, most JS/TS programmers will not be familiar with this pattern and the implementation here likely creates a hurtle to adoption.

Thanks for your work on this library, much appreciated!