Merge pull request #894 from chromaui/improve-upload-logging

Improve logging around upload errors

node-src/io/HTTPClient.ts

@@ -92,7 +92,8 @@ export default class HTTPClient {
           // You can only call text() or json() once, so if we are going to handle it outside of here..
           if (!opts.noLogErrorBody) {
             const body = await res.text();
-            this.log.debug({ body }, error.message);
+            this.log.debug(error.message);
+            this.log.debug(body);
           }
           throw error;
         }

node-src/lib/log.ts

@@ -35,6 +35,7 @@ export interface Logger {
   debug: LogFn;
   queue: () => void;
   flush: () => void;
+  getLevel: () => keyof typeof LOG_LEVELS;
   setLevel: (value: keyof typeof LOG_LEVELS) => void;
   setInteractive: (value: boolean) => void;
   setLogFile: (path: string | undefined) => void;
@@ -90,6 +91,9 @@ export const createLogger = () => {
     };
 
   const logger: Logger = {
+    getLevel() {
+      return level;
+    },
     setLevel(value: keyof typeof LOG_LEVELS) {
       if (value in LOG_LEVELS) level = value;
       else throw new Error(`Invalid level, expecting one of ${Object.keys(LOG_LEVELS).join(', ')}`);

node-src/lib/upload.ts

@@ -4,6 +4,7 @@ import { uploadFiles } from './uploadFiles';
 import { Context, FileDesc, TargetInfo } from '../types';
 import { maxFileCountExceeded } from '../ui/messages/errors/maxFileCountExceeded';
 import { maxFileSizeExceeded } from '../ui/messages/errors/maxFileSizeExceeded';
+import { uploadFailed } from '../ui/messages/errors/uploadFailed';
 
 // This limit is imposed by the uploadBuild mutation
 const MAX_FILES_PER_REQUEST = 1000;
@@ -174,7 +175,9 @@ export async function uploadBuild(
     ctx.uploadedBytes += totalBytes;
     ctx.uploadedFiles += targets.length;
   } catch (e) {
-    return options.onError?.(e, files.some((f) => f.localPath === e.message) && e.message);
+    const target = targets.find((target) => target.localPath === e.message);
+    if (target) ctx.log.error(uploadFailed({ target }, ctx.log.getLevel() === 'debug'));
+    return options.onError?.(e, target.localPath);
   }
 }
 

node-src/lib/uploadFiles.ts

@@ -18,7 +18,7 @@ export async function uploadFiles(
     targets.map(({ contentLength, filePath, formAction, formFields, localPath }) => {
       let fileProgress = 0; // The bytes uploaded for this this particular file
 
-      ctx.log.debug(`Uploading ${filePath} (${filesize(contentLength)})`);
+      ctx.log.debug(`Uploading ${filePath} (${filesize(contentLength)}) to ${formAction}`);
 
       return limitConcurrency(() =>
         retry(
@@ -37,17 +37,16 @@ export async function uploadFiles(
             Object.entries(formFields).forEach(([k, v]) => formData.append(k, v));
             formData.append('file', blob);
 
-            const res = await ctx.http.fetch(
-              formAction,
-              { body: formData, method: 'POST', signal },
-              { retries: 0 } // already retrying the whole operation
-            );
-
-            if (!res.ok) {
-              ctx.log.debug(`Uploading ${localPath} failed: %O`, res);
+            try {
+              await ctx.http.fetch(
+                formAction,
+                { body: formData, method: 'POST', signal },
+                { retries: 0 } // already retrying the whole operation
+              );
+              ctx.log.debug(`Uploaded ${filePath} (${filesize(contentLength)})`);
+            } catch (err) {
               throw new Error(localPath);
             }
-            ctx.log.debug(`Uploaded ${filePath} (${filesize(contentLength)})`);
           },
           {
             retries: ctx.env.CHROMATIC_RETRIES,

node-src/lib/waitForSentinel.ts

@@ -1,5 +1,4 @@
 import retry from 'async-retry';
-import { Response } from 'node-fetch';
 import { Context } from '../types';
 
 // A sentinel file is created by a zip-unpack lambda within the Chromatic infrastructure once the
@@ -18,23 +17,21 @@ export async function waitForSentinel(ctx: Context, { name, url }: { name: strin
         return bail(signal.reason || new Error('Aborted'));
       }
 
-      let res: Response;
       try {
-        res = await ctx.http.fetch(url, { signal }, { retries: 0, noLogErrorBody: true });
+        const res = await ctx.http.fetch(url, { signal }, { retries: 0 });
+        const result = await res.text();
+        if (result !== SENTINEL_SUCCESS_VALUE) {
+          ctx.log.debug(`Sentinel file '${name}' not OK, got '${result}'.`);
+          return bail(new Error(`Sentinel file '${name}' not OK.`));
+        }
+        ctx.log.debug(`Sentinel file '${name}' OK.`);
       } catch (e) {
         const { response = {} } = e;
         if (response.status === 403) {
           return bail(new Error('Provided signature expired.'));
         }
         throw new Error(`Sentinel file '${name}' not present.`);
       }
-
-      const result = await res.text();
-      if (result !== SENTINEL_SUCCESS_VALUE) {
-        ctx.log.debug(`Sentinel file '${name}' not OK, got '${result}'.`);
-        return bail(new Error(`Sentinel file '${name}' not OK.`));
-      }
-      ctx.log.debug(`Sentinel file '${name}' OK.`);
     },
     {
       retries: 185, // 3 minutes and some change (matches the lambda timeout with some extra buffer)

node-src/ui/messages/errors/uploadFailed.stories.ts

@@ -0,0 +1,20 @@
+import { uploadFailed } from './uploadFailed';
+
+export default {
+  title: 'CLI/Messages/Errors',
+};
+
+const target = {
+  contentLength: 12345,
+  localPath: 'local/path/to/file.js',
+  targetPath: 'file.js',
+  contentType: 'text/javascript',
+  fileKey: 'file-key',
+  filePath: 'file.js',
+  formAction: 'https://bucket.s3.amazonaws.com/',
+  formFields: { key: 'file-key', 'Content-Type': 'text/javascript' },
+};
+
+export const UploadFailed = () => uploadFailed({ target });
+
+export const UploadFailedDebug = () => uploadFailed({ target }, true);

node-src/ui/messages/errors/uploadFailed.ts

@@ -0,0 +1,20 @@
+import chalk from 'chalk';
+import { dedent } from 'ts-dedent';
+
+import { error as icon } from '../../components/icons';
+import { FileDesc, TargetInfo } from '../../../types';
+
+const encode = (path: string) => path.split('/').map(encodeURIComponent).join('/');
+
+export function uploadFailed({ target }: { target: FileDesc & TargetInfo }, debug = false) {
+  const diagnosis =
+    encode(target.targetPath) !== target.targetPath
+      ? 'It seems the file path may contain illegal characters.'
+      : 'The file may have been modified during the upload process.';
+  const message = dedent(chalk`
+    ${icon} Failed to upload {bold ${target.localPath}} to {bold ${target.targetPath}}
+    ${diagnosis}
+    ${debug ? '' : chalk`Enable the {bold debug} option to get more information.`}
+  `);
+  return debug ? message + JSON.stringify(target, null, 2) : message;
+}