supermacro / neverthrow

Type-Safe Errors for JS & TypeScript
MIT License
3.73k stars 78 forks source link

"Finally" functionality #525

Open scarabcoder opened 9 months ago

scarabcoder commented 9 months ago

I don't know if there have changes made since issue #254, but I am looking for the same functionality.

My specific use case is that I'm looking to avoid race conditions where concurrent calls are made to the same function, which operates with chained .andThen() calls. To do that, I'm using the library async-mutex, which lets you create a lock instance, then wait/lock/release it in situations where you don't want concurrency.

It hinges on the fact that no matter what, you are calling release(), so at the end of my chained andThen() calls it would be ideal to have it release() regardless of whether the ResultAsync ends up being a success or failure. However, I also don't want to modify the return type of the ResultAsync chain.

Currently, my workaround is to declare my AsyncResult chain as a variable, call .then() on that AsyncResult chain and have it call release() in both the failure and success callbacks, then return the original AsyncResult. That looks like this:

const result = ResultAsync.fromSafePromise(
      lock.acquire(),
    ).andThen(() => <additional logic>);

    // Ensure that the lock is released even if the transaction fails
    result.then(
      () => {
        lock.release();
      },
      () => {
        lock.release();
      },
    );

    return result;

This is a little annoying, the preferred syntax would be something like:

return ResultAsync.fromSafePromise(
      lock.acquire(),
    ).andThen(() => <additional logic>)
    .finally(() => lock.release());

Where finally wouldn't modify the error or success value types being returned.

scarabcoder commented 9 months ago

This solution is specific to my use case with async-mutex, but here's a util function I came up with to make the syntax a bit more developer-friendly (while also preserving types):

export const withLock = <T, E>(
  lock: Mutex,
  fn: () => ResultAsync<T, E>,
): ResultAsync<T, E> => {
  const result = ResultAsync.fromSafePromise(lock.acquire()).andThen(() => {
    return fn();
  });

  result.then(
    () => {
      lock.release();
    },
    () => {
      lock.release();
    },
  );
  return result;
};

Usage:

const result = await withLock(lock, () => doMyDbOperation()); // waits for a lock, locks, then runs doMyDbOperation(), before unlocking and returning the AsyncResult of doMyDBOperation
result.isOk();
result.value // the original response of doMyDbOperation
scarabcoder commented 9 months ago

If there's interest in some kind of finally() function, I could open up a PR to add it. I'm thinking finally() would be called with the final Result, and would be expected to NEVER throw/return an err value. Perhaps a name like safeFinally() would be more appropriate.

macksal commented 7 months ago

If there's interest in some kind of finally() function, I could open up a PR to add it. I'm thinking finally() would be called with the final Result, and would be expected to NEVER throw/return an err value. Perhaps a name like safeFinally() would be more appropriate.

I'd definitely make use of this.

There are perhaps some corner cases to consider - "cleanup" operations are often asynchronous themselves e.g. closing a database connection or websocket. Errors in the cleanup could also be important since if the cleanup fails, the surrounding method might disobey its usual contract.

This makes me think that finally would need to await the result of it's callback, and possibly even propagate errors that happen there. The result would be that finally works the same as andThen, but it is called for ok and error results, and if an error actually gets thrown somewhere in the chain.

This is similar to how the finally keyword works within async functions.

macksal commented 2 weeks ago

@scarabcoder I have added this feature in #588, please review if you are interested!