refactor: Better Result class implementation

lib/modules/datasource/rubygems/versions-endpoint-cache.ts

@@ -143,20 +143,20 @@ export class VersionsEndpointCache {
    */
   private async getCache(registryUrl: string): Promise<VersionsEndpointResult> {
     const oldResult = memCache.get(registryUrl);
-
     if (!oldResult) {
       const newResult = await this.fullSync(registryUrl);
       cacheResult(registryUrl, newResult);
       return newResult;
     }
 
-    if (!oldResult.res.success) {
+    const { value: data } = oldResult.unwrap();
+    if (!data) {
       return oldResult;
     }
 
-    if (isStale(oldResult.res.value)) {
+    if (isStale(data)) {
       memCache.delete(registryUrl); // If no error is thrown, we'll re-set the cache
-      const newResult = await this.deltaSync(oldResult.res.value, registryUrl);
+      const newResult = await this.deltaSync(data, registryUrl);
       cacheResult(registryUrl, newResult);
       return newResult;
     }
@@ -182,17 +182,17 @@ export class VersionsEndpointCache {
     } finally {
       this.cacheRequests.delete(registryUrl);
     }
-    const { res } = cachedResult;
 
-    if (!res.success) {
+    const { value: cachedData } = cachedResult.unwrap();
+    if (!cachedData) {
       logger.debug(
         { packageName, registryUrl },
         'Rubygems: endpoint not supported'
       );
       return Result.err('unsupported-api');
     }
 
-    const versions = res.value.packageVersions.get(packageName);
+    const versions = cachedData.packageVersions.get(packageName);
     if (!versions?.length) {
       logger.debug(
         { packageName, registryUrl },

lib/util/result.spec.ts

@@ -1,102 +1,178 @@
-import { Result } from './result';
+import { logger } from '../../test/util';
+import { AsyncResult, Result } from './result';
 
 describe('util/result', () => {
   describe('ok', () => {
     it('constructs successful result from value', () => {
-      expect(Result.ok(42).value).toBe(42);
+      expect(Result.ok(42).unwrap()).toEqual({ ok: true, value: 42 });
     });
   });
 
   describe('err', () => {
-    it('constructs `true` error by default', () => {
-      const res = Result.err();
-      expect(res.error).toBeTrue();
+    it('constructs error from value', () => {
+      expect(Result.err('oops').unwrap()).toEqual({ ok: false, error: 'oops' });
     });
 
-    it('constructs error result from Error instance', () => {
-      const err = new Error('oops');
-      const res = Result.err(err);
-      expect(res.error).toBe(err);
+    it('constructs error from Error instance', () => {
+      const error = new Error('oops');
+      expect(Result.err(error).unwrap()).toEqual({ ok: false, error });
     });
   });
 
   describe('wrap', () => {
     it('wraps function returning successful result', () => {
-      const res = Result.wrap(() => 42);
-      expect(res.value).toBe(42);
+      expect(Result.wrap(() => 42).unwrap(-1)).toBe(42);
     });
 
     it('wraps function that throws an error', () => {
-      const res = Result.wrap(() => {
-        throw new Error('oops');
-      });
-      expect(res.error?.message).toBe('oops');
+      expect(
+        Result.wrap(() => {
+          throw new Error('oops');
+        }).unwrap()
+      ).toEqual({ ok: false, error: new Error('oops') });
     });
 
     it('wraps promise resolving to value', async () => {
-      const res = await Result.wrap(Promise.resolve(42));
-      expect(res.value).toBe(42);
+      const res = await Result.wrap(Promise.resolve(42)).unwrap(-1);
+      expect(res).toBe(42);
     });
 
     it('wraps promise rejecting with error', async () => {
-      const err = new Error('oops');
-      const res = await Result.wrap(Promise.reject(err));
-      expect(res.error?.message).toBe('oops');
+      const res = await Result.wrap(Promise.reject('oops')).unwrap();
+      expect(res).toEqual({ ok: false, error: 'oops' });
     });
   });
 
   describe('transform', () => {
-    const fn = (x: string) => x.toUpperCase();
+    const fn = (x: string) => Result.ok(x.toUpperCase());
+    const afn = (x: string) => Promise.resolve(Result.ok(x.toUpperCase()));
 
     it('transforms successful result', () => {
       const res = Result.ok('foo').transform(fn);
       expect(res).toEqual(Result.ok('FOO'));
     });
 
-    it('no-op for error result', () => {
-      const err = new Error('bar');
-      const res = Result.err(err).transform(fn);
-      expect(res.value).toBeUndefined();
-      expect(res.error).toBe(err);
+    it('asynchronously transforms successful result', async () => {
+      const res = await Result.ok('foo').transform(afn);
+      expect(res).toEqual(Result.ok('FOO'));
     });
-  });
 
-  describe('catch', () => {
-    it('returns original value for successful result', () => {
-      const res = Result.ok(42);
-      expect(res.catch(0)).toBe(42);
+    it('bypasses transform for error result', async () => {
+      const res1 = Result.err('oops').transform(fn);
+      expect(res1).toEqual(Result.err('oops'));
+
+      const res2 = Result.err('oops').transform(afn);
+      expect(res2).toEqual(Result.err('oops'));
+
+      const res3 = await Result.wrap(Promise.reject('oops')).transform(fn);
+      expect(res3).toEqual(Result.err('oops'));
+
+      const res4 = await Result.wrap(Promise.reject('oops')).transform(afn);
+      expect(res4).toEqual(Result.err('oops'));
+
+      const res5 = await new AsyncResult<string, string>((_, reject) =>
+        reject('oops')
+      ).transform(afn);
+      expect(res5).toEqual(Result.err('oops'));
     });
 
-    it('returns fallback value for error result', () => {
-      const err = new Error('oops');
-      const res = Result.err(err);
-      expect(res.catch(42)).toBe(42);
+    it('logs and returns error for transform failure', () => {
+      const res = Result.ok('foo').transform(() => {
+        throw 'oops';
+      });
+      expect(res).toEqual(Result.err('oops'));
+      expect(logger.logger.warn).toHaveBeenCalledWith(
+        expect.anything(),
+        'Result: unhandled transform error'
+      );
     });
-  });
 
-  describe('value', () => {
-    it('returns successful value', () => {
-      const res = Result.ok(42);
-      expect(res.value).toBe(42);
+    it('logs and returns error for async transform failure', async () => {
+      const res = await Result.ok('foo').transform(() =>
+        Promise.reject('oops')
+      );
+      expect(res).toEqual(Result.err('oops'));
+      expect(logger.logger.warn).toHaveBeenCalledWith(
+        expect.anything(),
+        'Result: unhandled async transform error'
+      );
+    });
+
+    it('transforms successful promises', async () => {
+      const res = await Result.wrap(Promise.resolve('foo')).transform(fn);
+      expect(res).toEqual(Result.ok('FOO'));
     });
 
-    it('returns undefined value for error result', () => {
-      const err = new Error('oops');
-      const res = Result.err(err);
-      expect(res.value).toBeUndefined();
+    it('bypasses transform of failed promises', async () => {
+      const res = await Result.wrap(Promise.reject('bar')).transform(fn);
+      expect(res).toEqual(Result.err('bar'));
+    });
+
+    it('handles failed transform of successful promise', async () => {
+      const res = await Result.wrap(Promise.resolve('foo')).transform(() => {
+        throw 'bar';
+      });
+      expect(res).toEqual(Result.err('bar'));
+      expect(logger.logger.warn).toHaveBeenCalledWith(
+        expect.anything(),
+        'AsyncResult: unhandled transform error'
+      );
+    });
+
+    it('handles failed async transform of successful promise', async () => {
+      const res = await Result.wrap(Promise.resolve('foo')).transform(() =>
+        Promise.reject('bar')
+      );
+      expect(res).toEqual(Result.err('bar'));
+      expect(logger.logger.warn).toHaveBeenCalledWith(
+        expect.anything(),
+        'AsyncResult: unhandled async transform error'
+      );
+    });
+
+    it('handles chained failure', async () => {
+      const fn1 = (x: string): Result<string, string> =>
+        Result.ok(x.toUpperCase());
+      const fn2 = (x: string): Result<string[], number> =>
+        Result.ok(x.split(''));
+      const fn3 = (x: string[]): Result<string, boolean> =>
+        Result.ok(x.join(' '));
+
+      const res: Result<string, string | number | boolean> = await Result.wrap<
+        string,
+        string
+      >(Promise.resolve('foo'))
+        .transform(fn1)
+        .transform(fn2)
+        .transform(fn3);
+
+      expect(res).toEqual(Result.ok('F O O'));
     });
   });
 
-  describe('error', () => {
-    it('returns undefined error for successful result', () => {
+  describe('fallback', () => {
+    const fn = (x: string) => Result.ok(x.toUpperCase());
+
+    it('returns original value for successful result', () => {
       const res = Result.ok(42);
-      expect(res.error).toBeUndefined();
+      expect(res.unwrap(0)).toBe(42);
+    });
+
+    it('returns fallback value for error result', () => {
+      const res = Result.err(new Error('oops'));
+      expect(res.unwrap(42)).toBe(42);
+    });
+
+    it('skips fallback for successful promise transform', async () => {
+      const res = await Result.wrap(Promise.resolve('foo')).transform(fn);
+      expect(res).toEqual(Result.ok('FOO'));
     });
 
-    it('returns error for non-successful result', () => {
-      const err = new Error('oops');
-      const res = Result.err(err);
-      expect(res.error).toEqual(err);
+    it('uses fallback for failed promise transform', async () => {
+      const res = await Result.wrap(Promise.reject('foo'))
+        .transform(fn)
+        .unwrap('bar');
+      expect(res).toBe('bar');
     });
   });
 });