Skip to content

Commit

Permalink
Fix validation error messages in the browser (#9093)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kkaefer authored Dec 11, 2019
1 parent c9a9913 commit cfb63ae
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/source/video_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/style-spec/error/parsing_error.js
Original file line number Diff line number Diff line change
@@ -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;
}
Expand Down
9 changes: 6 additions & 3 deletions src/style-spec/error/validation_error.js
Original file line number Diff line number Diff line change
@@ -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__) {
Expand Down
8 changes: 6 additions & 2 deletions src/util/evented.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand Down
11 changes: 1 addition & 10 deletions test/unit/style-spec/validate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,14 @@ 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');
const style = fs.readFileSync(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();
});
});
Expand Down
11 changes: 1 addition & 10 deletions test/unit/style-spec/validate_mapbox_api_supported.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,14 @@ 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');
const style = fs.readFileSync(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();
});
});

0 comments on commit cfb63ae

Please sign in to comment.