Merge pull request #6 from mongodb-js/fix-error-messaging

fix: fix and improve error messaging

Author: gribnoysup

src/github-repo.spec.ts

@@ -323,7 +323,7 @@ describe('GithubRepo', () => {
     let deleteReleaseAsset: sinon.SinonStub;
 
     before(function() {
-      process.env.TEST_UPLOAD_RELEASE_ASSET_TIMEOUT = '1000';
+      process.env.TEST_UPLOAD_RELEASE_ASSET_TIMEOUT = '500';
     });
 
     after(function() {
@@ -457,6 +457,33 @@ describe('GithubRepo', () => {
       expect(githubRepo._uploadAsset).to.have.been.calledTwice;
     });
 
+    it('eventually fails if oktokit continues to fail', async function() {
+      const release = {
+        name: 'release',
+        tag: 'v0.8.0',
+        notes: '',
+      };
+
+      getReleaseByTag.resolves({});
+
+      githubRepo._uploadAsset = sinon
+        .stub()
+        .rejects({ message: 'request failed', status: 500 });
+
+      try {
+        await githubRepo.uploadReleaseAsset(release.tag, {
+          path: '/foo/bar/upload.zip',
+          contentType: 'xyz',
+        });
+        expect.fail('Expected uploadReleaseAsset to throw');
+      } catch (err) {
+        expect(err)
+          .to.have.property('message')
+          .match(/Failed to upload asset upload.zip after \d+ attempts/)
+          .match(/Octokit request failed/);
+      }
+    });
+
     it('fails if no release can be found', async() => {
       const release = {
         name: 'release',

src/github-repo.ts

@@ -73,32 +73,37 @@ async function waitFor<T>(
   fn: (...args: unknown[]) => Promise<T>,
   timeout = 60_000,
   interval = 100,
-  message: string | ((err: any) => string) = 'Timeout exceeded for task',
+  message:
+    | string
+    | ((err: any, attempts: number) => string) = 'Timeout exceeded for task',
   signal?: AbortSignal
 ): Promise<T> {
   let lastError: any;
+  let attempts = 0;
   const controller = new AbortController();
   // eslint-disable-next-line chai-friendly/no-unused-expressions
   signal?.addEventListener('abort', () => {
     controller.abort(signal.reason);
   });
   const tid = setTimeout(() => {
     controller.abort(
-      signal?.reason ??
-        new Error(typeof message === 'function' ? message(lastError) : message)
+      new Error(
+        typeof message === 'function' ? message(lastError, attempts) : message
+      )
     );
   }, timeout);
   try {
     while (!controller.signal.aborted) {
       try {
+        attempts++;
         return await fn();
       } catch (err) {
         lastError = err;
         await setTimeoutAsync(interval);
       }
     }
     // If we ended up here either timeout expired or passed signal was aborted,
-    // either way this inter
+    // either way this internal controller was aborted with a correct reason
     throw controller.signal.reason;
   } finally {
     clearTimeout(tid);
@@ -285,13 +290,16 @@ export class GithubRepo {
         },
         process.env.TEST_UPLOAD_RELEASE_ASSET_TIMEOUT
           ? Number(process.env.TEST_UPLOAD_RELEASE_ASSET_TIMEOUT)
-          // File upload is slow, we allow 5 minutes per file to allow for additional retries
-          : 60_000 * 5,
+          // File upload is slow, we allow 10 minutes per file to allow for additional retries
+          : 60_000 * 10,
         100,
-        (lastError) => {
-          return `Failed to upload asset ${asset.name}` + lastError
-            ? `\n\nLast encountered error:\n\n${lastError?.stack}`
-            : '';
+        (lastError, attempts) => {
+          return (
+            `Failed to upload asset ${path.basename(asset.path)} after ${attempts} attempts` +
+            (lastError
+              ? `\n\nLast encountered error:\n\n${this._getErrorMessage(lastError)}`
+              : '')
+          );
         },
         controller.signal
       );
@@ -325,10 +333,13 @@ export class GithubRepo {
         ? Number(process.env.TEST_GET_RELEASE_TIMEOUT)
         : 60_000,
       100,
-      (lastError) => {
-        return "Can't fetch releases from GitHub" + lastError
-          ? `\n\nLast encountered error:\n\n${lastError?.stack}`
-          : '';
+      (lastError, attempts) => {
+        return (
+          `Failed to fetch releases from GitHub after ${attempts} attempts` +
+          (lastError
+            ? `\n\nLast encountered error:\n\n${this._getErrorMessage(lastError)}`
+            : '')
+        );
       },
       controller.signal
     );
@@ -502,4 +513,17 @@ export class GithubRepo {
       error.errors[0].code === 'already_exists'
     );
   }
+
+  private _getErrorMessage(_err: any): string {
+    if (_err.status) {
+      return (
+        (_err as OctokitRequestError).errors
+          ?.map((err) => err.message ?? null)
+          .filter((msg): msg is string => !!msg)
+          .join('\n\n')
+          .trim() || `Octokit request failed with ${_err.name} (${_err.status})`
+      );
+    }
+    return _err.stack ?? _err.message;
+  }
 }