From cfb63ae531955c9d518967e50fb43cd5c2a7c0e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Wed, 11 Dec 2019 13:26:21 +0100 Subject: [PATCH] Fix validation error messages in the browser (#9093) We converted the ParsingError and ValidationError classes to inherit from the built-in Error class in #9025. This works fine in ES6, so our unit tests were happy. However, transpiling it down to ES5 leads to an absence of the message property, essentially removing the text of parsing and validation errors. --- src/source/video_source.js | 2 +- src/style-spec/error/parsing_error.js | 7 +++++-- src/style-spec/error/validation_error.js | 9 ++++++--- src/util/evented.js | 8 ++++++-- test/unit/style-spec/validate.test.js | 11 +---------- .../style-spec/validate_mapbox_api_supported.test.js | 11 +---------- 6 files changed, 20 insertions(+), 28 deletions(-) diff --git a/src/source/video_source.js b/src/source/video_source.js index a6222fe8a3f..a112419c2be 100644 --- a/src/source/video_source.js +++ b/src/source/video_source.js @@ -119,7 +119,7 @@ class VideoSource extends ImageSource { if (this.video) { const seekableRange = this.video.seekable; if (seconds < seekableRange.start(0) || seconds > seekableRange.end(0)) { - this.fire(new ErrorEvent(new ValidationError(`Playback for this video can be set only between the ${seekableRange.start(0)} and ${seekableRange.end(0)}-second mark.`))); + this.fire(new ErrorEvent(new ValidationError(`sources.${this.id}`, null, `Playback for this video can be set only between the ${seekableRange.start(0)} and ${seekableRange.end(0)}-second mark.`))); } else this.video.currentTime = seconds; } } diff --git a/src/style-spec/error/parsing_error.js b/src/style-spec/error/parsing_error.js index cd2cb6bce7f..8ddb923e2b3 100644 --- a/src/style-spec/error/parsing_error.js +++ b/src/style-spec/error/parsing_error.js @@ -1,12 +1,15 @@ // @flow -export default class ParsingError extends Error { +// Note: Do not inherit from Error. It breaks when transpiling to ES5. + +export default class ParsingError { + message: string; error: Error; line: number; constructor(error: Error) { - super(error.message); this.error = error; + this.message = error.message; const match = error.message.match(/line (\d+)/); this.line = match ? parseInt(match[1], 10) : 0; } diff --git a/src/style-spec/error/validation_error.js b/src/style-spec/error/validation_error.js index b57ad89b854..9b90c2f9800 100644 --- a/src/style-spec/error/validation_error.js +++ b/src/style-spec/error/validation_error.js @@ -1,11 +1,14 @@ // @flow -export default class ValidationError extends Error { +// Note: Do not inherit from Error. It breaks when transpiling to ES5. + +export default class ValidationError { + message: string; identifier: ?string; line: ?number; - constructor(key: string | null, value?: any, message?: string, identifier?: string) { - super([key, message].filter(a => a).join(': ')); + constructor(key: ?string, value: ?{ __line__: number }, message: string, identifier: ?string) { + this.message = (key ? `${key}: ` : '') + message; if (identifier) this.identifier = identifier; if (value !== null && value !== undefined && value.__line__) { diff --git a/src/util/evented.js b/src/util/evented.js index eeab1a88c79..9e6e438e966 100644 --- a/src/util/evented.js +++ b/src/util/evented.js @@ -31,10 +31,14 @@ export class Event { } } +interface ErrorLike { + message: string; +} + export class ErrorEvent extends Event { - error: Error; + error: ErrorLike; - constructor(error: Error, data: Object = {}) { + constructor(error: ErrorLike, data: Object = {}) { super('error', extend({error}, data)); } } diff --git a/test/unit/style-spec/validate.test.js b/test/unit/style-spec/validate.test.js index 2e37799d656..64fd3080d92 100644 --- a/test/unit/style-spec/validate.test.js +++ b/test/unit/style-spec/validate.test.js @@ -6,15 +6,6 @@ import validate from '../../../src/style-spec/validate_style'; const UPDATE = !!process.env.UPDATE; -function sanitizeError(error) { - const sanitized = {}; - sanitized.message = error.message; - for (const key in error) { - sanitized[key] = error[key]; - } - return sanitized; -} - glob.sync(`${__dirname}/fixture/*.input.json`).forEach((file) => { test(path.basename(file), (t) => { const outputfile = file.replace('.input', '.output'); @@ -22,7 +13,7 @@ glob.sync(`${__dirname}/fixture/*.input.json`).forEach((file) => { const result = validate(style); if (UPDATE) fs.writeFileSync(outputfile, JSON.stringify(result, null, 2)); const expect = JSON.parse(fs.readFileSync(outputfile)); - t.deepEqual(result.map(sanitizeError), expect); + t.deepEqual(result, expect); t.end(); }); }); diff --git a/test/unit/style-spec/validate_mapbox_api_supported.test.js b/test/unit/style-spec/validate_mapbox_api_supported.test.js index 37bd5e808ed..810eb111cb6 100644 --- a/test/unit/style-spec/validate_mapbox_api_supported.test.js +++ b/test/unit/style-spec/validate_mapbox_api_supported.test.js @@ -6,15 +6,6 @@ import validateMapboxApiSupported from '../../../src/style-spec/validate_mapbox_ const UPDATE = !!process.env.UPDATE; -function sanitizeError(error) { - const sanitized = {}; - sanitized.message = error.message; - for (const key in error) { - sanitized[key] = error[key]; - } - return sanitized; -} - glob.sync(`${__dirname}/fixture/*.input.json`).forEach((file) => { test(path.basename(file), (t) => { const outputfile = file.replace('.input', '.output-api-supported'); @@ -22,7 +13,7 @@ glob.sync(`${__dirname}/fixture/*.input.json`).forEach((file) => { const result = validateMapboxApiSupported(style); if (UPDATE) fs.writeFileSync(outputfile, JSON.stringify(result, null, 2)); const expect = JSON.parse(fs.readFileSync(outputfile)); - t.deepEqual(result.map(sanitizeError), expect); + t.deepEqual(result, expect); t.end(); }); });