Split errors into api and general

This commit is contained in:
Maciej Ziarkowski 2020-01-31 18:02:24 +00:00
parent 72cc7e62d2
commit fac4c8b35b
8 changed files with 110 additions and 60 deletions

View File

@ -2,7 +2,8 @@ import bodyParser from 'body-parser';
import express from 'express'; import express from 'express';
import * as editHistoryController from './controllers/editHistoryController'; import * as editHistoryController from './controllers/editHistoryController';
import { RequestParameterError, UserInputError } from './errors'; import { ApiParamError, ApiUserError } from './errors/api';
import { DatabaseError } from './errors/general';
import buildingsRouter from './routes/buildingsRouter'; import buildingsRouter from './routes/buildingsRouter';
import extractsRouter from './routes/extractsRouter'; import extractsRouter from './routes/extractsRouter';
import usersRouter from './routes/usersRouter'; import usersRouter from './routes/usersRouter';
@ -102,16 +103,18 @@ server.use((err: any, req: express.Request, res: express.Response, next: express
if (err != undefined) { if (err != undefined) {
console.log('Global error handler: ', err); console.log('Global error handler: ', err);
if (err instanceof UserInputError) { if (err instanceof ApiUserError) {
let errorMessage: string; let errorMessage: string;
if(err instanceof RequestParameterError) { if(err instanceof ApiParamError) {
errorMessage = `Problem with parameter ${err.paramName}: ${err.message}`; errorMessage = `Problem with parameter ${err.paramName}: ${err.message}`;
} else { } else {
errorMessage = err.message; errorMessage = err.message;
} }
res.status(400).send({ error: errorMessage }); res.status(400).send({ error: errorMessage });
} else if(err instanceof DatabaseError){
res.status(500).send({ error: 'Database error' });
} else { } else {
res.status(500).send({ error: 'Server error' }); res.status(500).send({ error: 'Server error' });
} }

View File

@ -1,6 +1,7 @@
import express from 'express'; import express from 'express';
import { UserInputError } from '../errors'; import { ApiParamError, ApiUserError } from '../errors/api';
import { ArgumentError } from '../errors/general';
import { checkRegexParam, parsePositiveIntParam, processParam } from '../parameters'; import { checkRegexParam, parsePositiveIntParam, processParam } from '../parameters';
import asyncController from "../routes/asyncController"; import asyncController from "../routes/asyncController";
import * as editHistoryService from '../services/editHistory'; import * as editHistoryService from '../services/editHistory';
@ -13,15 +14,19 @@ const getGlobalEditHistory = asyncController(async (req: express.Request, res: e
const count: number = processParam(req.query, 'count', parsePositiveIntParam); const count: number = processParam(req.query, 'count', parsePositiveIntParam);
if(afterId != undefined && beforeId != undefined) { if(afterId != undefined && beforeId != undefined) {
throw new UserInputError('Cannot specify both after_id and before_id parameters'); throw new ApiUserError('Cannot specify both after_id and before_id parameters');
} }
try { try {
const result = await editHistoryService.getGlobalEditHistory(beforeId, afterId, count); const result = await editHistoryService.getGlobalEditHistory(beforeId, afterId, count);
res.send(result); res.send(result);
} catch(error) { } catch(error) {
console.error(error); if(error instanceof ArgumentError && error.argumentName === 'count') {
res.send({ error: 'Database error' }); const apiErr = new ApiParamError(error.message, 'count');
throw apiErr;
}
throw error;
} }
}); });

View File

@ -1,5 +1,6 @@
import db from '../../db'; import db from '../../db';
import { EditHistoryEntry } from '../../frontend/models/edit-history-entry'; import { EditHistoryEntry } from '../../frontend/models/edit-history-entry';
import { DatabaseError } from '../errors/general';
const baseQuery = ` const baseQuery = `
SELECT SELECT
@ -19,53 +20,69 @@ export function getHistoryAfterId(id: string, count: number): Promise<EditHistor
* (like the other queries). The inner select is sorted in ascending order * (like the other queries). The inner select is sorted in ascending order
* so that the right rows are returned when limiting the result set. * so that the right rows are returned when limiting the result set.
*/ */
return db.manyOrNone(` try {
SELECT * FROM ( return db.any(`
${baseQuery} SELECT * FROM (
WHERE log_id > $1 ${baseQuery}
ORDER BY revision_id ASC WHERE log_id > $1
LIMIT $2 ORDER BY revision_id ASC
) AS result_asc ORDER BY revision_id DESC`, LIMIT $2
[id, count] ) AS result_asc ORDER BY revision_id DESC`,
); [id, count]
);
} catch(err) {
throw new DatabaseError(err);
}
} }
export function getHistoryBeforeId(id: string, count: number): Promise<EditHistoryEntry[]> { export function getHistoryBeforeId(id: string, count: number): Promise<EditHistoryEntry[]> {
if(id == undefined) { try {
if(id == undefined) {
return db.any(`
${baseQuery} return db.any(`
ORDER BY revision_id DESC ${baseQuery}
LIMIT $1 ORDER BY revision_id DESC
`, [count]); LIMIT $1
`, [count]);
} else {
} else {
return db.any(`
${baseQuery} return db.any(`
WHERE log_id < $1 ${baseQuery}
ORDER BY revision_id DESC WHERE log_id < $1
LIMIT $2 ORDER BY revision_id DESC
`, [id, count]); LIMIT $2
`, [id, count]);
}
} catch(err) {
throw new DatabaseError(err);
} }
} }
export async function getIdOlderThan(id: string): Promise<string> { export async function getIdOlderThan(id: string): Promise<string> {
const result = await db.oneOrNone<{revision_id:string}>(` try {
SELECT MAX(log_id) as revision_id const result = await db.oneOrNone<{revision_id:string}>(`
FROM logs SELECT MAX(log_id) as revision_id
WHERE log_id < $1 FROM logs
`, [id]); WHERE log_id < $1
`, [id]);
return result?.revision_id; return result?.revision_id;
} catch(err) {
throw new DatabaseError(err);
}
} }
export async function getIdNewerThan(id: string): Promise<string> { export async function getIdNewerThan(id: string): Promise<string> {
const result = await db.oneOrNone<{revision_id:string}>(` try {
SELECT MIN(log_id) as revision_id const result = await db.oneOrNone<{revision_id:string}>(`
FROM logs SELECT MIN(log_id) as revision_id
WHERE log_id > $1 FROM logs
`, [id]); WHERE log_id > $1
`, [id]);
return result?.revision_id; return result?.revision_id;
} catch(err) {
throw new DatabaseError(err);
}
} }

View File

@ -5,34 +5,40 @@
* https://stackoverflow.com/questions/41102060/typescript-extending-error-class * https://stackoverflow.com/questions/41102060/typescript-extending-error-class
*/ */
export class UserInputError extends Error { export class ApiUserError extends Error {
constructor(message?: string) { constructor(message?: string) {
super(message); super(message);
this.name = 'ApiUserError';
} }
} }
export class RequestParameterError extends UserInputError { export class ApiParamError extends ApiUserError {
public paramName: string; public paramName: string;
constructor(message?: string) { constructor(message?: string, paramName?: string) {
super(message); super(message);
this.name = 'ApiParamError';
this.paramName = paramName;
} }
} }
export class ParamRequiredError extends RequestParameterError { export class ApiParamRequiredError extends ApiParamError {
constructor(message?: string) { constructor(message?: string) {
super(message); super(message);
this.name = 'ApiParamRequiredError';
} }
} }
export class ParamOutOfBoundsError extends RequestParameterError { export class ApiParamOutOfBoundsError extends ApiParamError {
constructor(message?: string) { constructor(message?: string) {
super(message); super(message);
this.name = 'ApiParamOutOfBoundsError';
} }
} }
export class ParamInvalidFormatError extends RequestParameterError { export class ApiParamInvalidFormatError extends ApiParamError {
constructor(message?: string) { constructor(message?: string) {
super(message); super(message);
this.name = 'ApiParamInvalidFormatError';
} }
} }

View File

@ -0,0 +1,17 @@
export class ArgumentError extends Error {
public argumentName: string;
constructor(message?: string, argumentName?: string) {
super(message);
this.name = 'ArgumentError';
this.argumentName = argumentName;
}
}
export class DatabaseError extends Error {
public detail: any;
constructor(detail?: string) {
super();
this.name = 'DatabaseError';
this.detail = detail;
}
}

View File

@ -1,13 +1,13 @@
import { strictParseInt } from '../parse'; import { strictParseInt } from '../parse';
import { ParamInvalidFormatError, ParamRequiredError, RequestParameterError } from './errors'; import { ApiParamError, ApiParamInvalidFormatError, ApiParamRequiredError } from './errors/api';
export function processParam<T>(params: object, paramName: string, processingFn: (x: string) => T, required: boolean = false) { export function processParam<T>(params: object, paramName: string, processingFn: (x: string) => T, required: boolean = false) {
const stringValue = params[paramName]; const stringValue = params[paramName];
if(stringValue == undefined && required) { if(stringValue == undefined && required) {
const err = new ParamRequiredError('Parameter required but not supplied'); const err = new ApiParamRequiredError('Parameter required but not supplied');
err.paramName = paramName; err.paramName = paramName;
throw err; throw err;
} }
@ -15,7 +15,7 @@ export function processParam<T>(params: object, paramName: string, processingFn:
try { try {
return processingFn(stringValue); return processingFn(stringValue);
} catch(error) { } catch(error) {
if(error instanceof RequestParameterError) { if(error instanceof ApiParamError) {
error.paramName = paramName; error.paramName = paramName;
} }
@ -28,7 +28,7 @@ export function parsePositiveIntParam(param: string) {
const result = strictParseInt(param); const result = strictParseInt(param);
if (isNaN(result)) { if (isNaN(result)) {
throw new ParamInvalidFormatError('Invalid format: not a positive integer'); throw new ApiParamInvalidFormatError('Invalid format: not a positive integer');
} }
return result; return result;
} }
@ -37,7 +37,7 @@ export function checkRegexParam(param: string, regex: RegExp): string {
if(param == undefined) return undefined; if(param == undefined) return undefined;
if(param.match(regex) == undefined) { if(param.match(regex) == undefined) {
throw new ParamInvalidFormatError(`Invalid format: does not match regular expression ${regex}`); throw new ApiParamInvalidFormatError(`Invalid format: does not match regular expression ${regex}`);
} }
return param; return param;

View File

@ -1,6 +1,6 @@
import { EditHistoryEntry } from '../../../frontend/models/edit-history-entry'; import { EditHistoryEntry } from '../../../frontend/models/edit-history-entry';
import * as editHistoryData from '../../dataAccess/editHistory'; // manually mocked import * as editHistoryData from '../../dataAccess/editHistory'; // manually mocked
import { UserInputError } from '../../errors'; import { ArgumentError } from '../../errors/general';
import { getGlobalEditHistory } from '../editHistory'; import { getGlobalEditHistory } from '../editHistory';
jest.mock('../../dataAccess/editHistory'); jest.mock('../../dataAccess/editHistory');
@ -28,8 +28,10 @@ describe('getGlobalEditHistory()', () => {
[null, null], [null, null],
['100', null], ['100', null],
[null, '100'] [null, '100']
])('Should error when requesting non-positive number of records', (beforeId: string, afterId: string) => { ])('Should error when requesting non-positive number of records', async (beforeId: string, afterId: string) => {
expect(getGlobalEditHistory(beforeId, afterId, 0)).rejects.toBeInstanceOf(UserInputError); let resultPromise = getGlobalEditHistory(beforeId, afterId, 0);
await expect(resultPromise).rejects.toBeInstanceOf(ArgumentError);
await expect(resultPromise).rejects.toHaveProperty('argumentName', 'count');
}); });
describe('getting history before a point', () => { describe('getting history before a point', () => {

View File

@ -1,10 +1,10 @@
import { EditHistoryEntry } from '../../frontend/models/edit-history-entry'; import { EditHistoryEntry } from '../../frontend/models/edit-history-entry';
import { decBigInt, incBigInt } from '../../helpers'; import { decBigInt, incBigInt } from '../../helpers';
import { getHistoryAfterId, getHistoryBeforeId, getIdNewerThan, getIdOlderThan } from '../dataAccess/editHistory'; import { getHistoryAfterId, getHistoryBeforeId, getIdNewerThan, getIdOlderThan } from '../dataAccess/editHistory';
import { UserInputError } from '../errors'; import { ArgumentError } from '../errors/general';
async function getGlobalEditHistory(beforeId?: string, afterId?: string, count: number = 100) { async function getGlobalEditHistory(beforeId?: string, afterId?: string, count: number = 100) {
if(count <= 0) throw new UserInputError('cannot request less than 1 history record'); if(count <= 0) throw new ArgumentError('cannot request less than 1 history record', 'count');
if(count > 100) count = 100; if(count > 100) count = 100;
// limited set of records. Expected to be already ordered from newest to oldest // limited set of records. Expected to be already ordered from newest to oldest