Skip to content

Commit

Permalink
Merge pull request #15 from bitovi/feature/13-add-structured-errors
Browse files Browse the repository at this point in the history
Feature/13 add structured errors
  • Loading branch information
michael-dean-haynie committed Jun 10, 2022
2 parents 338967f + a38e7a0 commit e25aa5e
Show file tree
Hide file tree
Showing 20 changed files with 825 additions and 467 deletions.
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
},
// esbenp.prettier-vscode extension configuration
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.formatOnSave": true
"editor.formatOnSave": true,
"editor.tabSize": 2
}
12 changes: 12 additions & 0 deletions lib/errors/querystring-parsing-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/** Structured error for exceptions thrown during querystring parsing */
class QuerystringParsingError extends Error {
constructor({ message, querystring, paramKey, paramValue }) {
super(message);
this.querystring = querystring;
this.paramKey = paramKey;
this.paramValue = paramValue;
this.name = "QuerystringParsingError";
}
}

module.exports = QuerystringParsingError;
11 changes: 9 additions & 2 deletions lib/filter-styles/parse-ibm-filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const isNumberString = require("../helpers/is-number-string");
const isDateString = require("../helpers/is-date-string");
const isString = require("../helpers/is-string");
const IbmValueType = require("../enums/ibm-value-type");
const QuerystringParsingError = require("../../lib/errors/querystring-parsing-error");

/** Parses "IBM-style" filter expression from of a querystring. */
function parseIbmFilter(querystring) {
Expand All @@ -22,7 +23,14 @@ function parseIbmFilter(querystring) {
try {
subResults.push(parseExpression(expression));
} catch (e) {
errors.push(e);
errors.push(
new QuerystringParsingError({
message: e.message,
querystring,
paramKey: "filter",
paramValue: expression,
})
);
// break? all or nothing results?
}
}
Expand Down Expand Up @@ -96,7 +104,6 @@ function parseExpression(expression) {
}

function tokenizeExpression(expression) {
// TODO: what about string values with (),\' chars inside?
let tokens = [expression];
const delimiters = ["(", ")", ","];
delimiters.forEach((delim) => {
Expand Down
27 changes: 22 additions & 5 deletions lib/filter-styles/parse-mongo-filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const isNullString = require("../helpers/is-null-string");
const MongoOperator = require("../enums/mongo-operator");
const SqlOperator = require("../enums/sql-operator");
const MongoValueType = require("../enums/mongo-value-type");
const QuerystringParsingError = require("../../lib/errors/querystring-parsing-error");

/** Parses "MongoDB-style" filters from of a querystring. */
function parseMongoFilter(querystring) {
Expand Down Expand Up @@ -37,6 +38,18 @@ function parseMongoFilter(querystring) {
const operatorWasOmitted = providedOperator === undefined;
const providedValueWasAnArray = Array.isArray(providedValue);

// helper error constructor with pre-configured data for this field
const createError = (message) => {
return new QuerystringParsingError({
message,
querystring,
paramKey: `filter[${field}]${
operatorWasOmitted ? "" : "[" + providedOperator + "]"
}`,
paramValue: providedValue,
});
};

/************************************************************************
* 2. Apply defaults and type coersion
************************************************************************/
Expand All @@ -47,7 +60,7 @@ function parseMongoFilter(querystring) {
// verify all values are the same type (or null)
const valueType = areMongoTypesTheSame(values);
if (!valueType) {
errors.push(new Error("arrays should not mix multiple value types"));
errors.push(createError("arrays should not mix multiple value types"));
return { results, errors }; // short circuit
}

Expand Down Expand Up @@ -92,7 +105,9 @@ function parseMongoFilter(querystring) {
![MongoOperator.IN, MongoOperator.NOT_IN].includes(operator)
) {
errors.push(
new Error(`"${operator}" operator should not be used with array value`)
createError(
`"${operator}" operator should not be used with array value`
)
);
return { results, errors }; // short circuit
}
Expand All @@ -109,7 +124,7 @@ function parseMongoFilter(querystring) {
].includes(operator)
) {
errors.push(
new Error(`"${operator}" operator should not be used with null value`)
createError(`"${operator}" operator should not be used with null value`)
);
return { results, errors }; // short circuit
}
Expand All @@ -120,7 +135,7 @@ function parseMongoFilter(querystring) {
operator === MongoOperator.ILIKE
) {
errors.push(
new Error(
createError(
`"${operator}" operator should not be used with number values`
)
);
Expand All @@ -130,7 +145,9 @@ function parseMongoFilter(querystring) {
// date compatability check
if (valueType === MongoValueType.DATE && operator === MongoOperator.ILIKE) {
errors.push(
new Error(`"${operator}" operator should not be used with date values`)
createError(
`"${operator}" operator should not be used with date values`
)
);
return { results, errors }; // short circuit
}
Expand Down
6 changes: 5 additions & 1 deletion lib/helpers/determine-filter-style.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const FilterStyle = require("../enums/filter-style");
const isString = require("./is-string");
const QuerystringParsingError = require("../errors/querystring-parsing-error");

/** Determines which filter style a querystring employs */
function determineFilterStyle(querystring) {
Expand All @@ -8,7 +9,10 @@ function determineFilterStyle(querystring) {
const isIBM = querystring.includes("filter=");

if (isMongoDB && isIBM) {
throw new Error("querystring should not include multiple filter styles");
throw new QuerystringParsingError({
message: "querystring should not include multiple filter styles",
querystring,
});
} else if (isMongoDB) {
return FilterStyle.MONGO_DB;
} else if (isIBM) {
Expand Down
7 changes: 7 additions & 0 deletions lib/helpers/is-non-empty-string.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const isString = require("./is-string");

function isNonEmptyString(value) {
return isString(value) && value.length > 0;
}

module.exports = isNonEmptyString;
89 changes: 37 additions & 52 deletions lib/parse-fields.js
Original file line number Diff line number Diff line change
@@ -1,58 +1,54 @@
const qs = require("qs");
const isString = require("./helpers/is-string");
const isNonEmptyString = require("./helpers/is-non-empty-string");

const QuerystringParsingError = require("./errors/querystring-parsing-error");

function parseFields(querystring) {
const { fields: qsFields } = qs.parse(querystring);
const results = {};
const errors = [];

if (qsFields) {
for (const [type, fieldsInput] of Object.entries(qsFields)) {
let fieldsArray;
let params = qs.parse(querystring, { depth: 0, comma: true });
params = Object.entries(params).filter(([key]) => key.startsWith("fields"));

for (const [key, value] of params) {
// force array of values for simple logic
let values = Array.isArray(value) ? value : [value];

if (typeof fieldsInput === "string") {
// this is the default expected scenario
fieldsArray = fieldsInput.split(",");
} else if (Array.isArray(fieldsInput)) {
// this is what will happen if the type is specified twice like "fields[articles]=a&fields[articles]=b"
fieldsArray = fieldsInput;
} else {
// must assume the querystring did not follow JSON:API spec for sparse fieldsets
// See: https://jsonapi.org/format/#fetching-sparse-fieldsets
errors.push(
new Error(
`Incorrect format for fields in querystring: '${querystring}'`
)
);
}
// remove duplicates
values = values.reduce(removeDuplicatesReducer, []);

if (fieldsArray) {
// add errors for any duplicate fields
const duplicates = findDuplicates(fieldsArray);
duplicates.forEach((duplicate) => {
errors.push(
new Error(
`Duplicated field '${duplicate}' for type '${type}' detected in querystring: '${querystring}'`
)
);
});
// skip empty string values
values = values.filter(isNonEmptyString);
if (values.length === 0) {
continue;
}

// results
results[type] = fieldsArray
.filter(isNonEmptyString)
.reduce(removeDuplicatesReducer, []);
}
const type = getType(key);
if (!isNonEmptyString(type)) {
errors.push(
new QuerystringParsingError({
message: "Incorrect format was provided for fields.",
querystring,
paramKey: key,
paramValue: values,
})
);
continue;
}

results[type] = values;
}

return {
results,
errors,
};
return { results, errors };
}

function isNonEmptyString(value) {
return isString(value) && value.length;
/** Extracts the type from a fields query parameter. E.g. "cat" from "fields[cat]" */
function getType(param) {
const lBracket = param.indexOf("[");
const rBracket = param.indexOf("]");
if (lBracket !== -1 && rBracket !== -1) {
return param.slice(lBracket + 1, rBracket);
}
}

function removeDuplicatesReducer(accumulator, currentValue) {
Expand All @@ -62,15 +58,4 @@ function removeDuplicatesReducer(accumulator, currentValue) {
return accumulator;
}

function findDuplicates(input) {
const duplicates = [];
input.forEach((value) => {
if (input.filter((val) => val === value).length > 1) {
duplicates.push(value);
}
});

return [...new Set(duplicates)];
}

module.exports = parseFields;
4 changes: 2 additions & 2 deletions lib/parse-include.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const qs = require("qs");
const isNonEmptyString = require("./helpers/is-non-empty-string");

/**
* JSON:API specifies a list of "relationship paths"
Expand All @@ -10,9 +11,8 @@ function parseInclude(querystring) {
const results = [];
const errors = [];

// TODO: maybe need to check if this is a nonempty string (for all of them)
if (qsInclude) {
results.push(...qsInclude.split(","));
results.push(...qsInclude.split(",").filter(isNonEmptyString));
}

return {
Expand Down
43 changes: 26 additions & 17 deletions lib/parse-page.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const qs = require("qs");
const isString = require("./helpers/is-string");
const isNonEmptyString = require("./helpers/is-non-empty-string");
const QuerystringParsingError = require("./errors/querystring-parsing-error");

function parsePage(querystring) {
const { page: qsPage } = qs.parse(querystring);
Expand All @@ -13,15 +14,21 @@ function parsePage(querystring) {

if (numberWasProvided && !sizeWasProvided) {
errors.push(
new Error(
`Page number was provided but page size was not in querystring: '${querystring}'`
)
new QuerystringParsingError({
message: "Page number was provided but page size was not provided.",
querystring,
paramKey: "page[number]",
paramValue: number,
})
);
} else if (!numberWasProvided && sizeWasProvided) {
errors.push(
new Error(
`Page size was provided but page number was not in querystring: '${querystring}'`
)
new QuerystringParsingError({
message: "Page size was provided but page number was not provided.",
querystring,
paramKey: "page[size]",
paramValue: size,
})
);
} else {
const parsedNumber = parseInt(number);
Expand All @@ -32,16 +39,22 @@ function parsePage(querystring) {

if (!numberIsValid) {
errors.push(
new Error(
`Invalid page number provided in querystring: '${querystring}'`
)
new QuerystringParsingError({
message: "Invalid page number was provided.",
querystring,
paramKey: "page[number]",
paramValue: number,
})
);
}
if (!sizeIsValid) {
errors.push(
new Error(
`Invalid page size provided in querystring: '${querystring}'`
)
new QuerystringParsingError({
message: "Invalid page size was provided.",
querystring,
paramKey: "page[size]",
paramValue: size,
})
);
}

Expand All @@ -58,8 +71,4 @@ function parsePage(querystring) {
};
}

function isNonEmptyString(value) {
return isString(value) && value.length;
}

module.exports = parsePage;
Loading

0 comments on commit e25aa5e

Please sign in to comment.