refactor: Better tests for Result class (#23473)

lib/util/result.spec.ts

@@ -226,37 +226,37 @@ describe('util/result', () => {
 
     describe('Transforming', () => {
       it('transforms successful promise to value', async () => {
-        const res = await Result.wrap(Promise.resolve('foo')).transform((x) =>
+        const res = await AsyncResult.ok('foo').transform((x) =>
           x.toUpperCase()
         );
         expect(res).toEqual(Result.ok('FOO'));
       });
 
       it('transforms successful promise to Result', async () => {
-        const res = await Result.wrap(Promise.resolve('foo')).transform((x) =>
+        const res = await AsyncResult.ok('foo').transform((x) =>
           Result.ok(x.toUpperCase())
         );
         expect(res).toEqual(Result.ok('FOO'));
       });
 
       it('skips transform for failed promises', async () => {
-        const res = Result.wrap(Promise.reject<number>('oops'));
+        const res = AsyncResult.err('oops');
         const fn = jest.fn((x: number) => x + 1);
         await expect(res.transform(fn)).resolves.toEqual(Result.err('oops'));
         expect(fn).not.toHaveBeenCalled();
       });
 
       it('asyncronously transforms successfull promise to value', async () => {
-        const res = await Result.wrap(Promise.resolve('foo')).transform((x) =>
+        const res = await AsyncResult.ok('foo').transform((x) =>
           Promise.resolve(x.toUpperCase())
         );
         expect(res).toEqual(Result.ok('FOO'));
       });
 
-      it('asynchronously transforms successful Result promise to Result', async () => {
-        const res = await Result.wrap(
-          Promise.resolve(Result.ok('foo'))
-        ).transform((x) => Promise.resolve(Result.ok(x.toUpperCase())));
+      it('asynchronously transforms successful AsyncResult to Result', async () => {
+        const res = await AsyncResult.ok('foo').transform((x) =>
+          Promise.resolve(Result.ok(x.toUpperCase()))
+        );
         expect(res).toEqual(Result.ok('FOO'));
       });
 
@@ -283,20 +283,20 @@ describe('util/result', () => {
       });
 
       it('skips async transform for rejected promise', async () => {
-        const res: AsyncResult<number, string> = Result.wrap(
-          Promise.reject<number>('oops')
-        );
+        const res: AsyncResult<number, string> = AsyncResult.err('oops');
         const fn = jest.fn((x: number) => Promise.resolve(x + 1));
         await expect(res.transform(fn)).resolves.toEqual(Result.err('oops'));
         expect(fn).not.toHaveBeenCalled();
       });
 
-      it('skips async transform for error AsyncResult', async () => {
+      it('handles uncaught error from AsyncResult before transforming', async () => {
         const res: AsyncResult<number, string> = new AsyncResult((_, reject) =>
           reject('oops')
         );
         const fn = jest.fn((x: number) => Promise.resolve(x + 1));
-        await expect(res.transform(fn)).resolves.toEqual(Result.err('oops'));
+        await expect(res.transform(fn)).resolves.toEqual(
+          Result._uncaught('oops')
+        );
         expect(fn).not.toHaveBeenCalled();
       });
 
@@ -306,31 +306,31 @@ describe('util/result', () => {
           res.transform((_) => Promise.reject('oops'))
         ).resolves.toEqual(Result._uncaught('oops'));
         expect(logger.logger.warn).toHaveBeenCalledWith(
-          expect.anything(),
+          { err: 'oops' },
           'Result: unhandled async transform error'
         );
       });
 
       it('handles error thrown on promise transform', async () => {
-        const res = Result.wrap(Promise.resolve('foo'));
+        const res = AsyncResult.ok('foo');
         await expect(
           res.transform(() => {
             throw 'bar';
           })
         ).resolves.toEqual(Result._uncaught('bar'));
         expect(logger.logger.warn).toHaveBeenCalledWith(
-          expect.anything(),
+          { err: 'bar' },
           'AsyncResult: unhandled transform error'
         );
       });
 
       it('handles error thrown on promise async transform', async () => {
-        const res = Result.wrap(Promise.resolve('foo'));
+        const res = AsyncResult.ok('foo');
         await expect(
           res.transform(() => Promise.reject('bar'))
         ).resolves.toEqual(Result._uncaught('bar'));
         expect(logger.logger.warn).toHaveBeenCalledWith(
-          expect.anything(),
+          { err: 'bar' },
           'AsyncResult: unhandled async transform error'
         );
       });
@@ -346,9 +346,7 @@ describe('util/result', () => {
           Result.ok(x.join('-'));
 
         type Res = Result<string, string | number | boolean>;
-        const res: Res = await Result.wrap<string, string>(
-          Promise.resolve('foo')
-        )
+        const res: Res = await AsyncResult.ok('foo')
           .transform(fn1)
           .transform(fn2)
           .transform(fn3);

lib/util/result.ts

@@ -265,10 +265,10 @@ export class Result<T, E = Error> {
     fn: (
       value: NonNullable<T>
     ) =>
-      | NonNullable<U>
-      | Result<U, EE>
-      | Result<U, EE>
+      | Result<U, E | EE>
+      | Promise<Result<U, E | EE>>
       | Promise<NonNullable<U>>
+      | NonNullable<U>
   ): Result<U, E | EE> | AsyncResult<U, E | EE> {
     if (!this.res.ok) {
       return Result.err(this.res.err);
@@ -277,6 +277,10 @@ export class Result<T, E = Error> {
     try {
       const res = fn(this.res.val);
 
+      if (res instanceof Result) {
+        return res;
+      }
+
       if (res instanceof Promise) {
         return new AsyncResult((resolve) => {
           res
@@ -292,10 +296,6 @@ export class Result<T, E = Error> {
         });
       }
 
-      if (res instanceof Result) {
-        return res;
-      }
-
       return Result.ok(res);
     } catch (err) {
       logger.warn({ err }, 'Result: unhandled transform error');
@@ -320,6 +320,14 @@ export class AsyncResult<T, E> extends Promise<Result<T, E>> {
     super(executor);
   }
 
+  static ok<T>(val: NonNullable<T>): AsyncResult<T, never> {
+    return new AsyncResult((resolve) => resolve(Result.ok(val)));
+  }
+
+  static err<E>(err: NonNullable<E>): AsyncResult<never, E> {
+    return new AsyncResult((resolve) => resolve(Result.err(err)));
+  }
+
   static wrap<T, E = Error, EE = never>(
     promise: Promise<Result<T, EE>>
   ): AsyncResult<T, E | EE>;
@@ -429,7 +437,7 @@ export class AsyncResult<T, E> extends Promise<Result<T, E>> {
       | Promise<Result<U, EE>>
       | Promise<NonNullable<U>>
       | NonNullable<U>
-  ): AsyncResult<U, E | EE | Error> {
+  ): AsyncResult<U, E | EE> {
     return new AsyncResult((resolve) => {
       this.then((oldResult) => {
         const { ok, val: value, err: error } = oldResult.unwrap();
@@ -440,6 +448,10 @@ export class AsyncResult<T, E> extends Promise<Result<T, E>> {
         try {
           const newResult = fn(value);
 
+          if (newResult instanceof Result) {
+            return resolve(newResult);
+          }
+
           if (newResult instanceof Promise) {
             return newResult
               .then((asyncRes) =>
@@ -456,18 +468,14 @@ export class AsyncResult<T, E> extends Promise<Result<T, E>> {
               });
           }
 
-          if (newResult instanceof Result) {
-            return resolve(newResult);
-          }
-
           return resolve(Result.ok(newResult));
         } catch (err) {
           logger.warn({ err }, 'AsyncResult: unhandled transform error');
           return resolve(Result._uncaught(err));
         }
       }).catch((err) => {
-        // Actually, this should never happen
-        resolve(Result.err(err));
+        // Happens when `.unwrap()` of `oldResult` throws
+        resolve(Result._uncaught(err));
       });
     });
   }