Skip to content

Commit

Permalink
Slightly better error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@schmidt-sebastian
schmidt-sebastian committed Nov 12, 2018
1 parent 53da0e0 commit 9806572
Show file tree
Hide file tree
Showing 17 changed files with 272 additions and 213 deletions.
7 changes: 2 additions & 5 deletions dev/src/document.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ import {Timestamp} from './timestamp';
import {AnyDuringMigration, AnyJs, ApiMapValue, DocumentData, UpdateData, UserInput} from './types';

import api = google.firestore.v1beta1;
import {Validator} from './validate';
import Any = google.protobuf.Any;
import ArrayValue = google.firestore.v1beta1.ArrayValue;

/**
* Returns a builder for DocumentSnapshot and QueryDocumentSnapshot instances.
Expand Down Expand Up @@ -788,8 +785,8 @@ export class DocumentMask {
const result = applyDocumentMask(data);

if (result.remainingPaths.length !== 0) {
throw new Error(`Input data is missing for field '${
result.remainingPaths[0].toString()}'.`);
throw new Error(`Input data is missing for field "${
result.remainingPaths[0].toString()}".`);
}

return result.filteredData;
Expand Down
64 changes: 41 additions & 23 deletions dev/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,9 @@ export class Firestore {
*/
constructor(settings?: Settings) {
this._validator = new Validator({
ArrayElement: (name, value) =>
validateFieldValue(name, value, /* depth */ 0, /*inArray=*/true),
ArrayElement: (name, value) => validateFieldValue(
name, value, /* path */ undefined, /*level=*/0,
/*inArray=*/true),
DeletePrecondition: precondition =>
validatePrecondition(precondition, /* allowExists= */ true),
Document: validateDocumentData,
Expand Down Expand Up @@ -518,8 +519,8 @@ export class Firestore {
convertDocument = convert.documentFromJson;
} else {
throw new Error(
`Unsupported encoding format. Expected 'json' or 'protobufJS', ` +
`but was '${encoding}'.`);
`Unsupported encoding format. Expected "json" or "protobufJS", ` +
`but was "${encoding}".`);
}

const document = new DocumentSnapshotBuilder();
Expand Down Expand Up @@ -1269,51 +1270,67 @@ follow these steps, YOUR APP MAY BREAK.`);
*
* @private
* @param val JavaScript value to validate.
* @param path The field path to validate.
* @param options Validation options
* @param depth The current depth of the traversal.
* @param level The current depth of the traversal. This is used to decided
* whether deletes are allowed in conjunction with `allowDeletes: root`.
* @param inArray Whether we are inside an array.
* @returns 'true' when the object is valid.
* @throws when the object is invalid.
*/
function validateFieldValue(
val: AnyJs, options: ValidationOptions, depth?: number,
val: AnyJs, options: ValidationOptions, path?: FieldPath, level?: number,
inArray?: boolean): boolean {
if (!depth) {
depth = 1;
} else if (depth > MAX_DEPTH) {
if (path && path.size > MAX_DEPTH) {
throw new Error(
`Input object is deeper than ${MAX_DEPTH} levels or contains a cycle.`);
}

level = level || 0;
inArray = inArray || false;

if (is.array(val)) {
const fieldPathMessage = path ? ` (found in field ${path.toString()})` : '';

if (Array.isArray(val)) {
const arr = val as AnyDuringMigration[];
for (const prop of arr) {
validateFieldValue(arr[prop]!, options, depth! + 1, /* inArray= */ true);
for (let i = 0; i < arr.length; ++i) {
validateFieldValue(
arr[i]!, options,
path ? path.append(String(i)) : new FieldPath(String(i)), level + 1,
/* inArray= */ true);
}
} else if (isPlainObject(val)) {
const obj = val as object;
for (const prop in obj) {
if (obj.hasOwnProperty(prop)) {
validateFieldValue(obj[prop]!, options, depth! + 1, inArray);
validateFieldValue(
obj[prop]!, options,
path ? path.append(new FieldPath(prop)) : new FieldPath(prop),
level + 1, inArray);
}
}
} else if (val === undefined) {
throw new Error(
`Cannot use "undefined" as a Firestore value${fieldPathMessage}.`);
} else if (val instanceof DeleteTransform) {
if (inArray) {
throw new Error(`${val.methodName}() cannot be used inside of an array.`);
throw new Error(`${val.methodName}() cannot be used inside of an array${
fieldPathMessage}.`);
} else if (
(options.allowDeletes === 'root' && depth > 1) ||
(options.allowDeletes === 'root' && level !== 0) ||
options.allowDeletes === 'none') {
throw new Error(`${
val.methodName}() must appear at the top-level and can only be used in update() or set() with {merge:true}.`);
val.methodName}() must appear at the top-level and can only be used in update() or set() with {merge:true}${
fieldPathMessage}.`);
}
} else if (val instanceof FieldTransform) {
if (inArray) {
throw new Error(`${val.methodName}() cannot be used inside of an array.`);
throw new Error(`${val.methodName}() cannot be used inside of an array${
fieldPathMessage}.`);
} else if (!options.allowTransforms) {
throw new Error(`${
val.methodName}() can only be used in set(), create() or update().`);
throw new Error(
`${val.methodName}() can only be used in set(), create() or update()${
fieldPathMessage}.`);
}
} else if (val instanceof DocumentReference) {
return true;
Expand All @@ -1323,9 +1340,10 @@ function validateFieldValue(
return true;
} else if (val instanceof FieldPath) {
throw new Error(
'Cannot use object of type "FieldPath" as a Firestore value.');
`Cannot use object of type "FieldPath" as a Firestore value${
fieldPathMessage}.`);
} else if (is.object(val)) {
throw customObjectError(val);
throw customObjectError(val, path);
}

return true;
Expand All @@ -1343,7 +1361,7 @@ function validateFieldValue(
function validateDocumentData(
obj: DocumentData, options: ValidationOptions): boolean {
if (!isPlainObject(obj)) {
throw new Error('Input is not a plain JavaScript object.');
throw customObjectError(obj);
}

options = options || {};
Expand All @@ -1353,7 +1371,7 @@ function validateDocumentData(
for (const prop in obj) {
if (obj.hasOwnProperty(prop)) {
isEmpty = false;
validateFieldValue(obj[prop], options, /* depth= */ 1);
validateFieldValue(obj[prop], options, new FieldPath(prop));
}
}

Expand Down
51 changes: 32 additions & 19 deletions dev/src/path.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,28 @@ abstract class Path<T> {
*
* @private
* @hideconstructor
* @param {string[]} segments Sequence of parts of a path.
* @param segments Sequence of parts of a path.
*/
constructor(protected readonly segments: string[]) {}

/**
* String representation as expected by the proto API.
*
* @private
* @type {string}
*/
get formattedName(): string {
return this.canonicalString()!;
}

/**
* Returns the number of segments of this field path.
*
* @private
*/
get size(): number {
return this.segments.length;
}

abstract construct(segments: string[]|string): T;
abstract canonicalString(): string;
abstract split(relativePath: string): string[];
Expand All @@ -87,9 +95,8 @@ abstract class Path<T> {
* Create a child path beneath the current level.
*
* @private
* @param {string|T} relativePath Relative path to append to the current path.
* @returns {T} The new path.
* @template T
* @param relativePath Relative path to append to the current path.
* @returns The new path.
*/
append(relativePath: Path<T>|string): T {
if (relativePath instanceof Path) {
Expand All @@ -102,9 +109,7 @@ abstract class Path<T> {
* Returns the path of the parent node.
*
* @private
* @returns {T|null} The new path or null if we are already at the root.
* @returns {T} The new path.
* @template T
* @returns The new path or null if we are already at the root.
*/
parent(): T|null {
if (this.segments.length === 0) {
Expand All @@ -119,8 +124,7 @@ abstract class Path<T> {
*
* @private
* @param other The path to check against.
* @returns 'true' iff the current path is a prefix match with
* 'other'.
* @returns 'true' iff the current path is a prefix match with 'other'.
*/
isPrefixOf(other: Path<T>): boolean {
if (other.segments.length < this.segments.length) {
Expand All @@ -140,7 +144,7 @@ abstract class Path<T> {
* Returns a string representation of this path.
*
* @private
* @returns {string} A string representing this path.
* @returns A string representing this path.
*/
toString(): string {
return this.formattedName;
Expand Down Expand Up @@ -176,7 +180,7 @@ abstract class Path<T> {
* Returns a copy of the underlying segments.
*
* @private
* @returns {Array.<string>} A copy of the segments that make up this path.
* @returns A copy of the segments that make up this path.
*/
toArray(): string[] {
return this.segments.slice();
Expand All @@ -186,8 +190,8 @@ abstract class Path<T> {
* Returns true if this `Path` is equal to the provided value.
*
* @private
* @param {*} other The value to compare against.
* @return {boolean} true if this `Path` is equal to the provided value.
* @param other The value to compare against.
* @return true if this `Path` is equal to the provided value.
*/
isEqual(other: Path<T>): boolean {
return (
Expand Down Expand Up @@ -450,7 +454,7 @@ export class FieldPath extends Path<FieldPath> {
for (let i = 0; i < elements.length; ++i) {
validate.isString(i, elements[i]);
if (elements[i].length === 0) {
throw new Error(`Argument at index ${i} should not be empty.`);
throw new Error(`Element at index ${i} should not be an empty string.`);
}
}

Expand All @@ -477,20 +481,29 @@ export class FieldPath extends Path<FieldPath> {
*/
static validateFieldPath(fieldPath: string|FieldPath) {
if (!(fieldPath instanceof FieldPath)) {
if (!is.string(fieldPath)) {
if (fieldPath === undefined) {
throw new Error('Path cannot be ommitted.');
}

if (is.object(fieldPath) && fieldPath.constructor.name === 'FieldPath') {
throw customObjectError(fieldPath);
}

if (!is.string(fieldPath)) {
throw new Error(
'Paths can only be specified as strings or via a FieldPath object.');
}

if (fieldPath.indexOf('..') >= 0) {
throw new Error(`Paths must not contain '..' in them.`);
throw new Error(`Paths must not contain ".." in them.`);
}

if (fieldPath.startsWith('.') || fieldPath.endsWith('.')) {
throw new Error(`Paths must not start or end with '.'.`);
throw new Error(`Paths must not start or end with ".".`);
}

if (!FIELD_PATH_RE.test(fieldPath)) {
throw new Error(`Paths can't be empty and must not contain '*~/[]'.`);
throw new Error(`Paths can't be empty and must not contain "*~/[]".`);
}
}

Expand Down
12 changes: 6 additions & 6 deletions dev/src/reference.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -940,11 +940,11 @@ export class Query {
const fieldValue = documentSnapshot.get(fieldOrder.field);
if (fieldValue === undefined) {
throw new Error(
`Field '${
`Field "${
fieldOrder
.field}' is missing in the provided DocumentSnapshot. Please provide a ` +
'document that contains values for all specified orderBy() and ' +
'where() constraints.');
.field}" is missing in the provided DocumentSnapshot. ` +
'Please provide a document that contains values for all specified ' +
'orderBy() and where() constraints.');
} else {
fieldValues.push(fieldValue);
}
Expand Down Expand Up @@ -1319,7 +1319,7 @@ export class Query {
} else if (reference instanceof DocumentReference) {
if (!this._path.isPrefixOf(reference._path)) {
throw new Error(
`'${reference.path}' is not part of the query result set and ` +
`"${reference.path}" is not part of the query result set and ` +
'cannot be used as a query boundary.');
}
} else {
Expand All @@ -1331,7 +1331,7 @@ export class Query {
if (reference._path.parent()!.compareTo(this._path) !== 0) {
throw new Error(
'Only a direct child can be used as a query boundary. ' +
`Found: '${reference.path}'.`);
`Found: "${reference.path}".`);
}
return reference;
}
Expand Down
28 changes: 18 additions & 10 deletions dev/src/validate.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/

import * as is from 'is';

import {FieldPath} from './path';
import {AnyDuringMigration} from './types';

/**
Expand Down Expand Up @@ -130,7 +132,7 @@ export class Validator {
minNumberOfArguments(funcName, args, minSize): boolean {
if (args.length < minSize) {
throw new Error(
`Function '${funcName}()' requires at least ` +
`Function "${funcName}()" requires at least ` +
`${formatPlural(minSize, 'argument')}.`);
}

Expand All @@ -150,15 +152,17 @@ export class Validator {
maxNumberOfArguments(funcName, args, maxSize): boolean {
if (args.length > maxSize) {
throw new Error(
`Function '${funcName}()' accepts at most ` +
`Function "${funcName}()" accepts at most ` +
`${formatPlural(maxSize, 'argument')}.`);
}

return true;
}
}

export function customObjectError(val): Error {
export function customObjectError(val, path?: FieldPath): Error {
const fieldPathMessage = path ? ` (found in field ${path.toString()})` : '';

if (is.object(val) && val.constructor.name !== 'Object') {
const typeName = val.constructor.name;
switch (typeName) {
Expand All @@ -169,17 +173,21 @@ export function customObjectError(val): Error {
case 'Timestamp':
return new Error(
`Detected an object of type "${typeName}" that doesn't match the ` +
'expected instance. Please ensure that the Firestore types you ' +
'are using are from the same NPM package.');
`expected instance${fieldPathMessage}. Please ensure that the ` +
'Firestore types you are using are from the same NPM package.');
default:
return new Error(
`Couldn't serialize object of type "${typeName}". Firestore ` +
'doesn\'t support JavaScript objects with custom prototypes ' +
'(i.e. objects that were created via the \'new\' operator).');
`Couldn't serialize object of type "${typeName}"${
fieldPathMessage}. Firestore doesn't support JavaScript ` +
'objects with custom prototypes (i.e. objects that were created ' +
'via the "new" operator).');
}
} else if (!is.object(val)) {
throw new Error(
`Input is not a plain JavaScript object${fieldPathMessage}.`);
} else {
return new Error(
`Invalid use of type "${typeof val}" as a Firestore argument.`);
return new Error(`Invalid use of type "${
typeof val}" as a Firestore argument${fieldPathMessage}.`);
}
}

Expand Down
Loading

0 comments on commit 9806572

Please sign in to comment.