Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add andFinally functionality and tests #588

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/thirty-berries-complain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'neverthrow': minor
---

Add `andFinally` to allow for synchronous or asynchronous cleanup code that will always be called, and only propagates errors.
17 changes: 17 additions & 0 deletions src/result-async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,23 @@ export class ResultAsync<T, E> implements PromiseLike<Result<T, E>> {
)
}

andFinally<F>(f: () => ResultAsync<unknown, F> | Result<unknown, F>): ResultAsync<T, E | F> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing for either sync or async Results to be returned by the callback. I couldn't foresee any problems with this and it makes the method more flexible.

return new ResultAsync(
this._promise.then(
async (res) => {
const cleanupResult = await f()
return res.andFinally(() => cleanupResult)
},
async (err) => {
// this should be unreachable for well-behaved ResultAsync instances, but let's run the
// cleanup and propagate the erroneous rejection anyway
await f()
throw err
Comment on lines +174 to +177
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the PR description for more details about why this was added. Open to hearing concerns about this.

},
),
)
}

orElse<R extends Result<unknown, unknown>>(
f: (e: E) => R,
): ResultAsync<InferOkTypes<R> | T, InferErrTypes<R>>
Expand Down
30 changes: 30 additions & 0 deletions src/result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,18 @@ interface IResult<T, E> {
andThrough<R extends Result<unknown, unknown>>(f: (t: T) => R): Result<T, InferErrTypes<R> | E>
andThrough<F>(f: (t: T) => Result<unknown, F>): Result<T, E | F>

/**
* Similar to a `finally` block in throw-based code. Runs the given callback
* regardless of whether the result is an `Ok` or an `Err`.
*
* This is useful for cleanup operations that should always be run regardless of an error.
*
* An `Ok` returned from the callback will always be ignored, while any `Err`
* returned from the callback will be returned and replace the original `Err`
* if there was one. .
*/
andFinally<F>(f: () => Result<unknown, F>): Result<T, E | F>

/**
* Takes an `Err` value and maps it to a `Result<T, SomeNewType>`.
*
Expand Down Expand Up @@ -330,6 +342,15 @@ export class Ok<T, E> implements IResult<T, E> {
return ok<T, E>(this.value)
}

andFinally<F>(cleanup: () => Result<unknown, F>): Ok<T, never> | Err<never, F> {
const cleanupResult = cleanup()
if (cleanupResult.isErr()) {
return err(cleanupResult.error)
} else {
return ok(this.value)
}
}

orElse<R extends Result<unknown, unknown>>(
_f: (e: E) => R,
): Result<InferOkTypes<R> | T, InferErrTypes<R>>
Expand Down Expand Up @@ -420,6 +441,15 @@ export class Err<T, E> implements IResult<T, E> {
return err(this.error)
}

andFinally<F>(cleanup: () => Result<unknown, F>): Err<never, E | F> {
const cleanupResult = cleanup()
if (cleanupResult.isErr()) {
return err(cleanupResult.error)
} else {
return err(this.error)
}
}

orElse<R extends Result<unknown, unknown>>(
f: (e: E) => R,
): Result<InferOkTypes<R> | T, InferErrTypes<R>>
Expand Down
98 changes: 98 additions & 0 deletions tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,25 @@ describe('Result.Ok', () => {
})
})

describe('andFinally', () => {
it('calls the callback and returns the original ok value', () => {
const okVal = ok("original ok value");
const andFinallyFn = jest.fn(() => ok("finally ok value"));
const finalResult = okVal.andFinally(andFinallyFn);

expect(andFinallyFn).toHaveBeenCalledTimes(1);
expect(finalResult._unsafeUnwrap()).toBe("original ok value");
})
it('calls the callback and returns the error from the callback', () => {
const okVal = ok("original ok value");
const andFinallyFn = jest.fn(() => err("error from callback"));
const finalResult = okVal.andFinally(andFinallyFn);

expect(andFinallyFn).toHaveBeenCalledTimes(1);
expect(finalResult._unsafeUnwrapErr()).toBe("error from callback");
})
})

describe('asyncAndThrough', () => {
it('Calls the passed function but returns an original ok as Async', async () => {
const okVal = ok(12)
Expand Down Expand Up @@ -356,6 +375,25 @@ describe('Result.Err', () => {
expect(errVal._unsafeUnwrapErr()).toEqual('Yolo')
})

describe('andFinally', () => {
it('calls the callback and returns the original error', () => {
const okVal = err("original error");
const andFinallyFn = jest.fn(() => ok("finally ok value"));
const finalResult = okVal.andFinally(andFinallyFn);

expect(andFinallyFn).toHaveBeenCalledTimes(1);
expect(finalResult._unsafeUnwrapErr()).toBe("original error");
})
it('calls the callback and returns the error from the callback', () => {
const okVal = err("original error");
const andFinallyFn = jest.fn(() => err("error from callback"));
const finalResult = okVal.andFinally(andFinallyFn);

expect(andFinallyFn).toHaveBeenCalledTimes(1);
expect(finalResult._unsafeUnwrapErr()).toBe("error from callback");
})
})

it('Skips over asyncAndThrough but returns ResultAsync instead', async () => {
const errVal = err('Yolo')

Expand Down Expand Up @@ -1064,6 +1102,66 @@ describe('ResultAsync', () => {
})
})

describe('andFinally', () => {
it('runs the callback when the result is Ok and passes through the value', async () => {
const okVal = okAsync(42)
const finallyFn = jest.fn(() => {
return ok('this value is unused')
})
const finalResult = okVal.andFinally(finallyFn)
const awaitedResult = await finalResult

expect(finallyFn).toHaveBeenCalledTimes(1)
expect(finalResult).toBeInstanceOf(ResultAsync)
expect(awaitedResult._unsafeUnwrap()).toBe(42)
})
it('runs the callback when the result is Ok and returns an error from the callback', async () => {
const okVal = okAsync(42)
const finallyFn = jest.fn(() => {
return err('error from andFinally')
})
const finalResult = okVal.andFinally(finallyFn)
const awaitedResult = await finalResult

expect(finalResult).toBeInstanceOf(ResultAsync)
expect(finallyFn).toHaveBeenCalledTimes(1)
expect(awaitedResult._unsafeUnwrapErr()).toBe('error from andFinally')
})
it('runs the callback when the result is Err and passes through the error', async () => {
const errVal = errAsync('original error');
const finallyFn = jest.fn(() => {
return ok('this value is unused');
})
const finalResult = errVal.andFinally(finallyFn);
const awaitedResult = await finalResult;

expect(finalResult).toBeInstanceOf(ResultAsync);
expect(finallyFn).toHaveBeenCalledTimes(1);
expect(awaitedResult._unsafeUnwrapErr()).toBe('original error');
})
it('runs the callback when the result is Err and returns an error from the callback', async ()=>{
const errVal = errAsync('original error');
const finallyFn = jest.fn(() => {
return err('error from finally');
})
const finalResult = errVal.andFinally(finallyFn);
const awaitedResult = await finalResult;

expect(finalResult).toBeInstanceOf(ResultAsync);
expect(finallyFn).toHaveBeenCalledTimes(1);
expect(awaitedResult._unsafeUnwrapErr()).toBe('error from finally');
})
it('runs the callback when a misbehaving ResultAsync rejects, and propagates the rejection', async () => {
const misbehavingResult = new ResultAsync(Promise.reject('oops'));
const finallyFn = jest.fn(() => {
return ok('this value is unused');
})
const finalResult = misbehavingResult.andFinally(finallyFn);
await expect(finalResult).rejects.toBe('oops');
expect(finallyFn).toHaveBeenCalledTimes(1);
})
})

describe('orElse', () => {
it('Skips orElse on an Ok value', async () => {
const okVal = okAsync(12)
Expand Down