Skip to content

Commit

Permalink
chore(csv): Emit CSVViewer parse errors silently (#1062)
Browse files Browse the repository at this point in the history
* chore(csv): Emit CSVViewer parse errors silently

* chore: Update error code and sort errors

* chore: change to PAPAPARSE_ERROR_TYPES

* chore: using priority map

* chore: fix CSVViewer tests
  • Loading branch information
ConradJChan authored and mergify[bot] committed Aug 30, 2019
1 parent a41eba7 commit 5bb3491
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 16 deletions.
33 changes: 17 additions & 16 deletions src/lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,34 @@ export const VIEWER_EVENT = {
// Error codes logged by preview with "preview_error" events
export const ERROR_CODE = {
ACCOUNT: 'error_account',
UNSUPPORTED_FILE_TYPE: 'error_unsupported_file_type',
PERMISSIONS_PREVIEW: 'error_permissions_preview',
BAD_INPUT: 'error_bad_input',
BROWSER_GENERIC: 'error_browser_generic',
BROWSER_UNSUPPORTED: 'error_browser_unsupported',
CONTENT_DOWNLOAD: 'error_content_download',
CONVERSION_GENERIC: 'error_conversion_generic',
CONVERSION_PASSWORD_PROTECTED: 'error_password_protected',
CONVERSION_TRY_AGAIN_LATER: 'error_try_again_later',
CONVERSION_UNSUPPORTED_FORMAT: 'error_unsupported_format',
DELETED_REPS: 'error_deleted_reps',
EXCEEDED_RETRY_LIMIT: 'error_exceeded_retry_limit',
FLASH_NOT_ENABLED: 'error_flash_not_enabled',
GENERIC: 'error_generic',
IMAGE_SIZING: 'error_image_sizing',
INVALID_CACHE_ATTEMPT: 'error_invalid_file_for_cache',
LOAD_ANNOTATIONS: 'error_load_annotations',
LOAD_ASSET: 'error_load_asset',
LOAD_CSV: 'error_load_csv',
LOAD_DOCUMENT: 'error_load_document',
LOAD_MEDIA: 'error_load_media',
LOAD_VIEWER: 'error_load_viewer',
IMAGE_SIZING: 'error_image_sizing',
SHAKA: 'error_shaka',
INVALID_CACHE_ATTEMPT: 'error_invalid_file_for_cache',
NOT_DOWNLOADABLE: 'error_file_not_downloadable',
PARSE_CSV: 'error_parse_csv',
PERMISSIONS_PREVIEW: 'error_permissions_preview',
PREFETCH_FILE: 'error_prefetch_file',
RATE_LIMIT: 'error_rate_limit',
EXCEEDED_RETRY_LIMIT: 'error_exceeded_retry_limit',
BROWSER_GENERIC: 'error_browser_generic',
BROWSER_UNSUPPORTED: 'error_browser_unsupported',
NOT_DOWNLOADABLE: 'error_file_not_downloadable',
GENERIC: 'error_generic',
CONVERSION_GENERIC: 'error_conversion_generic',
CONVERSION_PASSWORD_PROTECTED: 'error_password_protected',
CONVERSION_TRY_AGAIN_LATER: 'error_try_again_later',
CONVERSION_UNSUPPORTED_FORMAT: 'error_unsupported_format',
SHAKA: 'error_shaka',
UNSUPPORTED_FILE_TYPE: 'error_unsupported_file_type',
VIEWER_LOAD_TIMEOUT: 'error_viewer_load_timeout',
CONTENT_DOWNLOAD: 'error_content_download',
FLASH_NOT_ENABLED: 'error_flash_not_enabled',
};

// Event fired from Preview with error details
Expand Down
43 changes: 43 additions & 0 deletions src/lib/viewers/text/CSVViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ import { ERROR_CODE, VIEWER_EVENT } from '../../events';
import PreviewError from '../../PreviewError';

const JS = [`third-party/text/${TEXT_STATIC_ASSETS_VERSION}/papaparse.min.js`, 'csv.js'];
const PAPAPARSE_ERROR_TYPES = {
DELIMITER: 'Delimiter',
FIELD_MISMATCH: 'FieldMismatch',
QUOTES: 'Quotes',
};
const ERROR_PRIORITY = {
[PAPAPARSE_ERROR_TYPES.DELIMITER]: 0,
[PAPAPARSE_ERROR_TYPES.QUOTES]: 1,
[PAPAPARSE_ERROR_TYPES.FIELD_MISMATCH]: 2,
};

class CSVViewer extends TextBaseViewer {
/**
Expand Down Expand Up @@ -67,6 +77,9 @@ class CSVViewer extends TextBaseViewer {
if (this.isDestroyed() || !results) {
return;
}

this.checkForParseErrors(results);

this.data = results.data;
this.finishLoading();
URL.revokeObjectURL(workerSrc);
Expand All @@ -77,6 +90,36 @@ class CSVViewer extends TextBaseViewer {
.catch(this.handleAssetError);
}

/**
* Checks for parse errors and if present triggers an error silently
* @param {Array} results.errors Papaparse results errors array
* @return {void}
*/
checkForParseErrors({ errors = [] } = {}) {
if (!errors.length) {
return;
}

const parseError = this.getWorstParseError(errors);

const error = new PreviewError(ERROR_CODE.PARSE_CSV, undefined, {
...parseError,
silent: true,
});

this.triggerError(error);
}

/**
* Utility to sort the PapaParse errors by most significant and returning the first.
* The significance is defined as DELIMTER > QUOTES > FIELD_MISMATCH
* @param {Array} errors Array of errors from PapaParse parse results
* @return {Object} returns PapaParse error or undefined
*/
getWorstParseError(errors = []) {
return errors.sort((a, b) => ERROR_PRIORITY[a.type] - ERROR_PRIORITY[b.type])[0];
}

/**
* Prefetches assets for CSV Viewer.
*
Expand Down
51 changes: 51 additions & 0 deletions src/lib/viewers/text/__tests__/CSVViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,55 @@ describe('lib/viewers/text/CSVViewer', () => {
expect(csv.emit).to.be.calledWith(VIEWER_EVENT.load);
});
});

describe('checkForParseErrors()', () => {
beforeEach(() => {
stubs.getWorstParseError = sandbox.stub(csv, 'getWorstParseError');
stubs.triggerError = sandbox.stub(csv, 'triggerError');
});

it('should do nothing if no errors', () => {
csv.checkForParseErrors();

expect(stubs.triggerError).not.to.be.called;
});

it('should trigger error with a parse error', () => {
stubs.getWorstParseError.returns({ foo: 'bar' });

csv.checkForParseErrors({ errors: [{ foo: 'bar' }] });

expect(stubs.triggerError).to.be.called;
});
});

describe('getWorstParseError()', () => {
const delimiterError = { type: 'Delimiter' };
const fieldsMismatchError = { type: 'FieldMismatch', id: 1 };
const fieldsMismatchError2 = { type: 'FieldMismatch', id: 2 };
const quotesError = { type: 'Quotes' };

[
{ name: 'should return undefined if empty array', errors: [], expectedError: undefined },
{
name: 'should return delimiter type if present',
errors: [fieldsMismatchError, delimiterError, quotesError],
expectedError: delimiterError,
},
{
name: 'should return quotes type if no delimiter type in array',
errors: [fieldsMismatchError, quotesError],
expectedError: quotesError,
},
{
name: 'should return fields mismatch type if no other type present',
errors: [fieldsMismatchError, fieldsMismatchError2],
expectedError: fieldsMismatchError,
},
].forEach(({ name, errors, expectedError }) => {
it(`${name}`, () => {
expect(csv.getWorstParseError(errors)).to.be.eql(expectedError);
});
});
});
});

0 comments on commit 5bb3491

Please sign in to comment.