Merge pull request #5046 from betagouv/signal-network-errors

Usager : amélioration des erreurs de l'envoi automatique des pièces jointes

app/javascript/new_design/dossiers/auto-upload-controller.js

@@ -1,6 +1,6 @@
 import Uploader from '../../shared/activestorage/uploader';
-import ProgressBar from '../../shared/activestorage/progress-bar';
+import { show, hide, toggle } from '@utils';
-import { ajax, show, hide, toggle } from '@utils';
+import { FAILURE_CONNECTIVITY } from '../../shared/activestorage/file-upload-error';
 
 // Given a file input in a champ with a selected file, upload a file,
 // then attach it to the dossier.
@@ -11,27 +11,21 @@ export default class AutoUploadController {
   constructor(input, file) {
     this.input = input;
     this.file = file;
+    this.uploader = new Uploader(
+      input,
+      file,
+      input.dataset.directUploadUrl,
+      input.dataset.autoAttachUrl
+    );
   }
 
+  // Create, upload and attach the file.
+  // On failure, display an error message and throw a FileUploadError.
   async start() {
     try {
       this._begin();
-
+      await this.uploader.start();
-      // Sanity checks
+      this._succeeded();
-      const autoAttachUrl = this.input.dataset.autoAttachUrl;
-      if (!autoAttachUrl) {
-        throw new Error('L’attribut "data-auto-attach-url" est manquant');
-      }
-
-      // Upload the file (using Direct Upload)
-      let blobSignedId = await this._upload();
-
-      // Attach the blob to the champ
-      // (The request responds with Javascript, which displays the attachment HTML fragment).
-      await this._attach(blobSignedId, autoAttachUrl);
-
-      // Everything good: clear the original file input value
-      this.input.value = null;
     } catch (error) {
       this._failed(error);
       throw error;
@@ -45,35 +39,8 @@ export default class AutoUploadController {
     this._hideErrorMessage();
   }
 
-  async _upload() {
+  _succeeded() {
-    const uploader = new Uploader(
+    this.input.value = null;
-      this.input,
-      this.file,
-      this.input.dataset.directUploadUrl
-    );
-    return await uploader.start();
-  }
-
-  async _attach(blobSignedId, autoAttachUrl) {
-    // Now that the upload is done, display a new progress bar
-    // to show that the attachment request is still pending.
-    const progressBar = new ProgressBar(
-      this.input,
-      `${this.input.id}-progress-bar`,
-      this.file
-    );
-    progressBar.progress(100);
-    progressBar.end();
-
-    const attachmentRequest = {
-      url: autoAttachUrl,
-      type: 'PUT',
-      data: `blob_signed_id=${blobSignedId}`
-    };
-    await ajax(attachmentRequest);
-
-    // The progress bar has been destroyed by the attachment HTML fragment that replaced the input,
-    // so no further cleanup is needed.
   }
 
   _failed(error) {
@@ -81,56 +48,39 @@ export default class AutoUploadController {
       return;
     }
 
-    let progressBar = this.input.parentElement.querySelector('.direct-upload');
+    this.uploader.progressBar.destroy();
-    if (progressBar) {
-      progressBar.remove();
-    }
 
-    this._displayErrorMessage(error);
+    let message = this._messageFromError(error);
+    this._displayErrorMessage(message);
   }
 
   _done() {
     this.input.disabled = false;
   }
 
-  _isError422(error) {
-    // Ajax errors have an xhr attribute
-    if (error && error.xhr && error.xhr.status == 422) return true;
-    // Rails DirectUpload errors are returned as a String, e.g. 'Error creating Blob for "Demain.txt". Status: 422'
-    if (error && error.toString().includes('422')) return true;
-
-    return false;
-  }
-
   _messageFromError(error) {
-    let allowRetry = !this._isError422(error);
+    let message = error.message || error.toString();
+    let canRetry = error.status && error.status != 422;
 
-    if (
+    if (error.failureReason == FAILURE_CONNECTIVITY) {
-      error.xhr &&
-      error.xhr.status == 422 &&
-      error.response &&
-      error.response.errors &&
-      error.response.errors[0]
-    ) {
       return {
-        title: error.response.errors[0],
+        title: 'Le fichier n’a pas pu être envoyé.',
-        description: '',
+        description: 'Vérifiez votre connexion à Internet, puis ré-essayez.',
-        retry: allowRetry
+        retry: true
       };
     } else {
       return {
-        title: 'Une erreur s’est produite pendant l’envoi du fichier.',
+        title: 'Le fichier n’a pas pu être envoyé.',
-        description: error.message || error.toString(),
+        description: message,
-        retry: allowRetry
+        retry: canRetry
       };
     }
   }
 
-  _displayErrorMessage(error) {
+  _displayErrorMessage(message) {
     let errorNode = this.input.parentElement.querySelector('.attachment-error');
     if (errorNode) {
       show(errorNode);
-      let message = this._messageFromError(error);
       errorNode.querySelector('.attachment-error-title').textContent =
         message.title || '';
       errorNode.querySelector('.attachment-error-description').textContent =

app/javascript/new_design/dossiers/auto-uploads-controllers.js

@@ -1,5 +1,6 @@
 import Rails from '@rails/ujs';
 import AutoUploadController from './auto-upload-controller.js';
+import { FAILURE_CONNECTIVITY } from '../../shared/activestorage/file-upload-error';
 
 // Manage multiple concurrent uploads.
 //
@@ -17,6 +18,11 @@ export default class AutoUploadsControllers {
     try {
       let controller = new AutoUploadController(input, file);
       await controller.start();
+    } catch (error) {
+      // Report errors to Sentry (except connectivity issues)
+      if (error.failureReason != FAILURE_CONNECTIVITY) {
+        throw error;
+      }
     } finally {
       this._decrementInFlightUploads(form);
     }

app/javascript/shared/activestorage/errors.js

@@ -1,22 +0,0 @@
-// Convert an error message returned by DirectUpload to a proper error object.
-//
-// This function has two goals:
-// 1. Remove the file name from the DirectUpload error message
-//   (because the filename confuses Sentry error grouping)
-// 2. Create each kind of error on a different line
-//   (so that Sentry knows they are different kind of errors, from
-//   the line they were created.)
-export default function errorFromDirectUploadMessage(message) {
-  let matches = message.match(/ Status: [0-9]{1,3}/);
-  let status = (matches && matches[0]) || '';
-
-  if (message.includes('Error creating')) {
-    return new Error('Error creating file.' + status);
-  } else if (message.includes('Error storing')) {
-    return new Error('Error storing file.' + status);
-  } else if (message.includes('Error reading')) {
-    return new Error('Error reading file.' + status);
-  } else {
-    return new Error(message);
-  }
-}

app/javascript/shared/activestorage/file-upload-error.js

@@ -0,0 +1,67 @@
+// Error while reading the file client-side
+export const ERROR_CODE_READ = 'file-upload-read-error';
+// Error while creating the empty blob on the server
+export const ERROR_CODE_CREATE = 'file-upload-create-error';
+// Error while uploading the blob content
+export const ERROR_CODE_STORE = 'file-upload-store-error';
+// Error while attaching the blob to the record
+export const ERROR_CODE_ATTACH = 'file-upload-attach-error';
+
+// Failure on the client side (syntax error, error reading file, etc.)
+export const FAILURE_CLIENT = 'file-upload-failure-client';
+// Failure on the server side (typically non-200 response)
+export const FAILURE_SERVER = 'file-upload-failure-server';
+// Failure during the transfert (request aborted, connection lost, etc)
+export const FAILURE_CONNECTIVITY = 'file-upload-failure-connectivity';
+
+/**
+  Represent an error during a file upload.
+  */
+export default class FileUploadError extends Error {
+  constructor(message, status, code) {
+    super(message);
+    this.name = 'FileUploadError';
+    this.status = status;
+    this.code = code;
+  }
+
+  /**
+    Return the component responsible of the error (client, server or connectivity).
+    See FAILURE_* constants for values.
+    */
+  get failureReason() {
+    let isNetworkError = this.code != ERROR_CODE_READ;
+
+    if (isNetworkError && this.status != 0) {
+      return FAILURE_SERVER;
+    } else if (isNetworkError && this.status == 0) {
+      return FAILURE_CONNECTIVITY;
+    } else {
+      return FAILURE_CLIENT;
+    }
+  }
+}
+
+// Convert an error message returned by DirectUpload to a proper error object.
+//
+// This function has two goals:
+// 1. Remove the file name from the DirectUpload error message
+//   (because the filename confuses Sentry error grouping)
+// 2. Create each kind of error on a different line
+//   (so that Sentry knows they are different kind of errors, from
+//   the line they were created.)
+export function errorFromDirectUploadMessage(message) {
+  let matches = message.match(/ Status: [0-9]{1,3}/);
+  let status = (matches && parseInt(matches[0], 10)) || undefined;
+
+  // prettier-ignore
+  if (message.includes('Error reading')) {
+    return new FileUploadError('Error reading file.', status, ERROR_CODE_READ);
+  } else if (message.includes('Error creating')) {
+    return new FileUploadError('Error creating file.', status, ERROR_CODE_CREATE);
+  } else if (message.includes('Error storing')) {
+    return new FileUploadError('Error storing file.', status, ERROR_CODE_STORE);
+  } else {
+    return new FileUploadError(message, status, undefined);
+  }
+}

app/javascript/shared/activestorage/ujs.js

@@ -1,5 +1,8 @@
 import ProgressBar from './progress-bar';
-import errorFromDirectUploadMessage from './errors';
+import {
+  errorFromDirectUploadMessage,
+  FAILURE_CONNECTIVITY
+} from './file-upload-error';
 import { fire } from '@utils';
 
 const INITIALIZE_EVENT = 'direct-upload:initialize';
@@ -56,7 +59,9 @@ addUploadEventListener(ERROR_EVENT, event => {
   ProgressBar.error(id, errorMsg);
 
   let error = errorFromDirectUploadMessage(errorMsg);
+  if (error.failureReason != FAILURE_CONNECTIVITY) {
     fire(document, 'sentry:capture-exception', error);
+  }
 });
 
 addUploadEventListener(END_EVENT, ({ detail: { id } }) => {

app/javascript/shared/activestorage/uploader.js

@@ -1,35 +1,88 @@
 import { DirectUpload } from '@rails/activestorage';
+import { ajax } from '@utils';
 import ProgressBar from './progress-bar';
-import errorFromDirectUploadMessage from './errors';
+import FileUploadError, {
+  errorFromDirectUploadMessage,
+  ERROR_CODE_ATTACH
+} from './file-upload-error';
 
 /**
   Uploader class is a delegate for DirectUpload instance
   used to track lifecycle and progress of an upload.
   */
 export default class Uploader {
-  constructor(input, file, directUploadUrl) {
+  constructor(input, file, directUploadUrl, autoAttachUrl) {
     this.directUpload = new DirectUpload(file, directUploadUrl, this);
     this.progressBar = new ProgressBar(input, this.directUpload.id, file);
+    this.autoAttachUrl = autoAttachUrl;
   }
 
-  start() {
+  /**
+    Upload (and optionally attach) the file.
+    Returns the blob signed id on success.
+    Throws a FileUploadError on failure.
+    */
+  async start() {
     this.progressBar.start();
 
+    try {
+      let blobSignedId = await this._upload();
+
+      if (this.autoAttachUrl) {
+        await this._attach(blobSignedId);
+      }
+
+      this.progressBar.end();
+      this.progressBar.destroy();
+
+      return blobSignedId;
+    } catch (error) {
+      this.progressBar.error(error.message);
+      throw error;
+    }
+  }
+
+  /**
+    Upload the file using the DirectUpload instance, and return the blob signed_id.
+    Throws a FileUploadError on failure.
+    */
+  async _upload() {
     return new Promise((resolve, reject) => {
       this.directUpload.create((errorMsg, attributes) => {
         if (errorMsg) {
-          this.progressBar.error(errorMsg);
           let error = errorFromDirectUploadMessage(errorMsg);
           reject(error);
         } else {
           resolve(attributes.signed_id);
         }
-        this.progressBar.end();
-        this.progressBar.destroy();
       });
     });
   }
 
+  /**
+    Attach the file by sending a POST request to the autoAttachUrl.
+    Throws a FileUploadError on failure (containing the first validation
+    error message, if any).
+    */
+  async _attach(blobSignedId) {
+    const attachmentRequest = {
+      url: this.autoAttachUrl,
+      type: 'PUT',
+      data: `blob_signed_id=${blobSignedId}`
+    };
+
+    try {
+      await ajax(attachmentRequest);
+    } catch (e) {
+      let message = e.response && e.response.errors && e.response.errors[0];
+      throw new FileUploadError(
+        message || 'Error attaching file.',
+        e.xhr.status,
+        ERROR_CODE_ATTACH
+      );
+    }
+  }
+
   uploadRequestDidProgress(event) {
     const progress = (event.loaded / event.total) * 100;
     if (progress) {