Skip to content

Commit

Permalink
Implement better error handling from the backend
Browse files Browse the repository at this point in the history
  • Loading branch information
@j-maynard
j-maynard committed Jun 14, 2024
1 parent 72f4ed1 commit bf4d33e
Show file tree
Hide file tree
Showing 13 changed files with 228 additions and 86 deletions.
107 changes: 78 additions & 29 deletions src/app.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import Backend from 'i18next-fs-backend';
import i18nextMiddleware from 'i18next-http-middleware';
import multer from 'multer';

// eslint-disable-next-line import/no-cycle
import { API } from './controllers/api';
import { healthcheck } from './route/healthcheck';
import { FileList } from './dtos/filelist';
import { ViewErrDTO } from './dtos/view-dto';

i18next
.use(Backend)
Expand All @@ -30,13 +32,18 @@ i18next
debug: false
});

export const i18n = i18next;
export const t = i18next.t;
export const ENGLISH = 'en-GB';
export const WELSH = 'cy-GB';

export const logger = pino({
name: 'StatsWales-Alpha-App',
level: 'debug'
});

const app: Application = express();
const APIInstance = new API(logger);
const APIInstance = new API();
const storage = multer.memoryStorage();
const upload = multer({ storage });

Expand Down Expand Up @@ -74,18 +81,28 @@ app.get('/:lang/publish/name', (req: Request, res: Response) => {
app.post('/:lang/publish/name', upload.none(), (req: Request, res: Response) => {
if (!req.body?.internal_name) {
logger.debug('Internal name was missing on request');
res.status(400);
res.render('publish/name', {
const err: ViewErrDTO = {
success: false,
headers: undefined,
data: undefined,
status: 400,
dataset_id: undefined,
errors: [
{
field: 'internal_name',
message: 'No dataset name provided'
message: [
{
lang: req.i18n.language,
message: t('errors.name_missing')
}
],
tag: {
name: 'errors.name_missing',
params: {}
}
}
]
});
};
res.status(400);
res.render('publish/name', err);
return;
}
const internalName: string = req.body.internal_name;
Expand All @@ -96,37 +113,56 @@ app.post('/:lang/publish/upload', upload.single('csv'), async (req: Request, res
const lang = req.params.lang;
if (!req.body?.internal_name) {
logger.debug('Internal name was missing on request');
res.status(400);
res.render('publish/name', {
const err: ViewErrDTO = {
success: false,
headers: undefined,
data: undefined,
status: 400,
dataset_id: undefined,
errors: [
{
field: 'internal_name',
message: 'No dataset name provided'
message: [
{
lang: req.i18n.language,
message: t('errors.name_missing')
}
],
tag: {
name: 'errors.name_missing',
params: {}
}
}
]
});
};
res.status(400);
res.render('publish/name', err);
return;
}
logger.debug(`Internal name: ${req.body.internal_name}`);
const internalName: string = req.body.internal_name;
if (!req.file) {
logger.debug('Attached file was missing on this request');
res.status(400);
res.render('publish/upload', {
const err: ViewErrDTO = {
success: false,
headers: undefined,
data: undefined,
internal_name: internalName,
status: 400,
dataset_id: undefined,
errors: [
{
field: 'csv',
message: 'No CSV data available'
message: [
{
lang: req.i18n.language,
message: t('errors.upload.no-csv-data')
}
],
tag: {
name: 'errors.upload.no-csv-data',
params: {}
}
}
]
});
};
res.status(400);
res.render('publish/upload', err);
return;
}

Expand All @@ -147,29 +183,42 @@ app.get('/:lang/list', async (req: Request, res: Response) => {
res.render('list', fileList);
});

app.get('/:lang/data', async (req: Request, res: Response) => {
app.get('/:lang/data/:file', async (req: Request, res: Response) => {
const lang = req.params.lang;
const page_number: number = Number.parseInt(req.query.page_number as string, 10) || 1;
const page_size: number = Number.parseInt(req.query.page_size as string, 10) || 100;

if (!req.query.file) {
res.status(400);
res.render('data', {
if (!req.params.file) {
const err: ViewErrDTO = {
success: false,
headers: undefined,
data: undefined,
status: 404,
dataset_id: undefined,
errors: [
{
field: 'file',
message: 'No filename provided'
message: [
{
lang: req.i18n.language,
message: t('errors.dataset_missing')
}
],
tag: {
name: 'errors.dataset_missing',
params: {}
}
}
]
});
};
res.status(404);
res.render('data', err);
return;
}

const file_id = req.query.file.toString();
const file_id = req.params.file;
const file = await APIInstance.getFileData(lang, file_id, page_number, page_size);
if (!file.success) {
res.status((file as ViewErrDTO).status);
}
res.render('data', file);
});

Expand Down
110 changes: 97 additions & 13 deletions src/controllers/api.ts
View file
Original file line number Diff line number Diff line change
@@ -1,35 +1,59 @@
import { env } from 'process';

import { Logger } from 'pino';

import { FileList } from '../dtos/filelist';
import { ProcessedCSV } from '../dtos/processedcsv-dto';
// eslint-disable-next-line import/no-cycle
import { logger } from '../app';
import { FileListError, FileList } from '../dtos/filelist';
import { ViewDTO, ViewErrDTO } from '../dtos/view-dto';
import { Healthcheck } from '../dtos/healthcehck';
import { UploadDTO, UploadErrDTO } from '../dtos/upload-dto';

class HttpError extends Error {
public status: number;

constructor(status: number) {
super('');
this.status = status;
}

async handleMessage(message: Promise<string>) {
const msg = await message;
this.message = msg;
}
}

export class API {
private readonly backend_server: string;
private readonly backend_port: string;
private readonly backend_protocol: string;
private readonly logger: Logger;

constructor(logger: Logger) {
constructor() {
this.backend_server = env.BACKEND_SERVER || 'localhost';
this.backend_port = env.BACKEND_PORT || '3001';
if (env.BACKEND_PROTOCOL === 'https') {
this.backend_protocol = 'https';
} else {
this.backend_protocol = 'http';
}
this.logger = logger;
}

public async getFileList(lang: string) {
const filelist: FileList = await fetch(
`${this.backend_protocol}://${this.backend_server}:${this.backend_port}/${lang}/dataset`
)
.then((api_res) => api_res.json())
.then((response) => {
if (response.ok) {
return response.json();
}
const err = new HttpError(response.status);
err.handleMessage(response.text());
throw err;
})
.then((api_res) => {
return api_res as FileList;
})
.catch((error) => {
logger.error(`An HTTP error occured with status ${error.status} and message "${error.message}"`);
return { status: error.status, files: [], error: error.message } as FileListError;
});
return filelist;
}
Expand All @@ -38,9 +62,39 @@ export class API {
const file = await fetch(
`${this.backend_protocol}://${this.backend_server}:${this.backend_port}/${lang}/dataset/${file_id}/view?page_number=${page_number}&page_size=${page_size}`
)
.then((api_res) => api_res.json())
.then((response) => {
if (response.ok) {
return response.json();
}
const err = new HttpError(response.status);
err.handleMessage(response.text());
throw err;
})
.then((api_res) => {
return api_res as ProcessedCSV;
return api_res as ViewDTO;
})
.catch((error) => {
logger.error(`An HTTP error occured with status ${error.status} and message "${error.message}"`);
return {
success: false,
status: error.status,
errors: [
{
field: 'file',
message: [
{
lang,
message: 'errors.dataset_missing'
}
],
tag: {
name: 'errors.dataset_missing',
params: {}
}
}
],
dataset_id: file_id
} as ViewErrDTO;
});
return file;
}
Expand All @@ -50,16 +104,46 @@ export class API {
formData.append('csv', file, filename);
formData.append('internal_name', filename);

const processedCSV: ProcessedCSV = await fetch(
const processedCSV = await fetch(
`${this.backend_protocol}://${this.backend_server}:${this.backend_port}/${lang}/dataset/`,
{
method: 'POST',
body: formData
}
)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
Loading
of this request depends on a
user-provided value
Loading
.
.then((api_res) => api_res.json())
.then((response) => {
if (response.ok) {
return response.json();
}
const err = new HttpError(response.status);
err.handleMessage(response.text());
throw err;
})
.then((api_res) => {
return api_res as ProcessedCSV;
return api_res as UploadDTO;
})
.catch((error) => {
logger.error(`An HTTP error occured with status ${error.status} and message "${error.message}"`);
return {
success: false,
status: error.status,
errors: [
{
field: 'csv',
message: [
{
lang,
message: 'errors.upload.no-csv-data'
}
],
tag: {
name: 'errors.upload.no-csv-data',
params: {}
}
}
],
dataset: undefined
} as UploadErrDTO;
});
return processedCSV;
}
Expand Down
11 changes: 10 additions & 1 deletion src/dtos/error.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
export interface ErrorMessage {
lang: string;
message: string;
}

export interface Error {
field: string;
message: string;
message: ErrorMessage[];
tag: {
name: string;
params: object;
};
}
6 changes: 6 additions & 0 deletions src/dtos/filelist.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,9 @@ export interface FileDescription {
export interface FileList {
files: FileDescription[];
}

export interface FileListError {
status: number;
files: FileDescription[];
error: string;
}
21 changes: 0 additions & 21 deletions src/dtos/processedcsv-dto.ts
View file

This file was deleted.

Loading

0 comments on commit bf4d33e

Please sign in to comment.