javascript: improve Sentry grouping of direct upload errors

DirectUpload returns errors as strings, including an HTTP status and a
file name (and without a stack trace).

But Sentry groups issues according to the stack trace, and maybe the
error message in last resort.

So we have an issue: as all DirectUpload errors logged by Sentry are
generated on the same line, with random-looking messages, Sentry groups
them either too or too little aggressively.

Instead of creating all the errors on the same line:

- add some `if`s statements to create them on different lines (and so
  with different stack traces),
- strip the file name from the error message.

This allows Sentry to group the errors properly, with meaningful error
messages.
This commit is contained in:
Pierre de La Morinerie 2020-04-09 16:45:31 +02:00
parent 499808a384
commit c633cd0888
3 changed files with 34 additions and 4 deletions

View file

@ -0,0 +1,22 @@
// 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);
}
}

View file

@ -1,4 +1,5 @@
import ProgressBar from './progress-bar';
import errorFromDirectUploadMessage from './errors';
import { fire } from '@utils';
const INITIALIZE_EVENT = 'direct-upload:initialize';
@ -40,17 +41,22 @@ addUploadEventListener(PROGRESS_EVENT, ({ detail: { id, progress } }) => {
});
addUploadEventListener(ERROR_EVENT, event => {
let id = event.detail.id;
let errorMsg = event.detail.error;
// Display an error message
alert(
`Nous sommes désolés, une erreur sest produite lors de lenvoi du fichier.
(${event.detail.error})`
(${errorMsg})`
);
// Prevent ActiveStorage from displaying its own error message
event.preventDefault();
ProgressBar.error(event.detail.id, event.detail.error);
fire(document, 'sentry:capture-exception', new Error(event.detail.error));
ProgressBar.error(id, errorMsg);
let error = errorFromDirectUploadMessage(errorMsg);
fire(document, 'sentry:capture-exception', error);
});
addUploadEventListener(END_EVENT, ({ detail: { id } }) => {

View file

@ -1,5 +1,6 @@
import { DirectUpload } from '@rails/activestorage';
import ProgressBar from './progress-bar';
import errorFromDirectUploadMessage from './errors';
/**
Uploader class is a delegate for DirectUpload instance
@ -18,7 +19,8 @@ export default class Uploader {
this.directUpload.create((errorMsg, attributes) => {
if (errorMsg) {
this.progressBar.error(errorMsg);
reject(new Error(errorMsg));
let error = errorFromDirectUploadMessage(errorMsg);
reject(error);
} else {
resolve(attributes.signed_id);
}