Skip to content

Commit

Permalink
refactor(db): improve error reporting and code clean up (#171)
Browse files Browse the repository at this point in the history 
* fix(db): do not hide real db errors and do not print databse in error since it can contains username and password

* chore(db): add @slangroom/shared as dependency

* refactor(db): improve error reporting

Remove also some duplicated code

* test(db): more failing test

* fix(db): improve handling of connectionRefusedError error

the message field of that error was an empty string

* fix(db): improve typing

* test(db): refactor and remove some useless try...catch
  • Loading branch information
@matteo-cristino
matteo-cristino authored Jul 1, 2024
1 parent de0850a commit 2c76af0
Show file tree
Hide file tree
Showing 5 changed files with 215 additions and 252 deletions.
1 change: 1 addition & 0 deletions pkg/db/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"version": "1.33.13",
"dependencies": {
"@slangroom/core": "workspace:*",
"@slangroom/shared": "workspace:*",
"sequelize": "^6.16.0",
"sqlite3": "^5.0.0"
},
Expand Down
163 changes: 89 additions & 74 deletions pkg/db/src/plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,20 @@
// SPDX-License-Identifier: AGPL-3.0-or-later

import { Plugin } from '@slangroom/core';
import { BindOrReplacements, DataTypes, Model, Sequelize } from "sequelize";
import { JsonableObject } from '@slangroom/shared';
import {
QueryOptions,
ConnectionRefusedError,
BindOrReplacements,
DataTypes,
Model,
Sequelize
} from "sequelize";
// read the version from the package.json
import packageJson from '@slangroom/db/package.json' with { type: 'json' };

class Result extends Model {
public result!: string;
public result: string = "";
}

export class DbError extends Error {
Expand All @@ -20,13 +28,47 @@ export class DbError extends Error {

type DK = string | object | null | undefined;

function safeJSONParse(o: DK, errorMessage?: string): ({ ok: true, parsed: object } | { ok: false, error: string }) {
const safeJSONParse = (o: DK):
({ ok: true, parsed: JsonableObject } | { ok: false, error: string }) => {
const notNull = o ?? {};
if (typeof notNull === "object") return { ok: true, parsed: notNull };
if (typeof notNull === "object") return { ok: true, parsed: notNull as JsonableObject };
try {
return { ok: true, parsed: JSON.parse(notNull) };
} catch (err) {
return { ok: false, error: errorMessage ?? err };
return { ok: false, error: err.message };
}
}

const safeDbInit = (database: string):
({ ok: true, db: Sequelize } | { ok: false, error: string }) => {
try {
const urlParts = new URL(database);
if (!urlParts.protocol.endsWith(':'))
throw new Error('invalid url, it must start with "dialect://"');
const db = new Sequelize(database, { logging: false });
return { ok: true, db };
} catch (err) {
return { ok: false, error: err.message }
}
}

const safeDbQuery = async (db: Sequelize, statement: string, params?: BindOrReplacements):
Promise<{ ok: true, res: any } | { ok: false, error: string }> => {

Check failure on line 56 in pkg/db/src/plugin.ts

View workflow job for this annotation

GitHub Actions / build_and_test (lts/*) 

Unexpected any. Specify a different type

Check failure on line 56 in pkg/db/src/plugin.ts

View workflow job for this annotation

GitHub Actions / build_and_test (22.2.0) 

Unexpected any. Specify a different type
try {
const t = await db.transaction();
const opt: QueryOptions = {
transaction: t
};
if (params) opt.replacements = params;
const [o, m] = await db.query(statement, opt);
await t.commit();
return { ok: true, res: o ? o : m };
} catch (err) {
if (err instanceof ConnectionRefusedError)
return { ok: false, error: 'connection refused' };
return { ok: false, error: err.message };
} finally {
db.close();
}
}

Expand All @@ -47,21 +89,12 @@ export const execute = p.new('connect',
async (ctx) => {
const statement = ctx.fetch('statement') as string;
const database = ctx.fetchConnect()[0] as string;
try {
const db = new Sequelize(database, {
// disable logging; default: console.log
logging: false
});
const t = await db.transaction();
const [o, m] = await db.query(statement, { transaction: t });
await t.commit();
const output: any = o ? o : m;
db.close();
return ctx.pass(output);
} catch (error) {
return ctx.fail(new DbError(error.message));
}
},
const initRes = safeDbInit(database);
if (!initRes.ok) return ctx.fail(new DbError(initRes.error));
const queryRes = await safeDbQuery(initRes.db, statement);
if (!queryRes.ok) return ctx.fail(new DbError(queryRes.error));
return ctx.pass(queryRes.res);
}
);

/**
Expand All @@ -75,21 +108,12 @@ export const executeParams = p.new('connect',
const statement = ctx.fetch('statement') as string;
const parameters = ctx.fetch('parameters') as BindOrReplacements;
const database = ctx.fetchConnect()[0] as string;
try {
const db = new Sequelize(database, { logging: false });
const t = await db.transaction();
const [o, m] = await db.query(statement, {
transaction: t,
replacements: parameters
});
await t.commit();
const output: any = o ? o : m;
db.close();
return ctx.pass(output);
} catch (error) {
return ctx.fail(new DbError(error.message));
}
},
const initRes = safeDbInit(database);
if (!initRes.ok) return ctx.fail(new DbError(initRes.error));
const queryRes = await safeDbQuery(initRes.db, statement, parameters);
if (!queryRes.ok) return ctx.fail(new DbError(queryRes.error));
return ctx.pass(queryRes.res);
}
);

/**
Expand All @@ -107,12 +131,11 @@ export const getRecord = p.new('connect',
const record = ctx.fetch('record') as string;
const table = ctx.fetch('table') as string;
const database = ctx.fetchConnect()[0] as string;

const parse = (o: string) => safeJSONParse(o, `[DATABASE] Error in JSON format "${o}"`)
const initRes = safeDbInit(database)
if (!initRes.ok) return ctx.fail(new DbError(initRes.error));
const db = initRes.db;

try {
var output = {}
const db = new Sequelize(database, { logging: false });
Result.init(
{ result: DataTypes.TEXT },
{
Expand All @@ -122,28 +145,22 @@ export const getRecord = p.new('connect',
}
);
await Result.sync();
try {
let result = await Result.findByPk(record);
if (result) {
result = result.get({ plain: true });
// column name is result
const resultData = parse(result!.result);
if (!resultData.ok) return ctx.fail(new DbError(resultData.error));
output = resultData.parsed;
} else {
return ctx.fail(new DbError(`[DATABASE]
Returned null for id "${record}" in table "${table}" in db "${database}".`));
}
} catch (e) {
return ctx.fail(new DbError(`[DATABASE]
Something went wrong for id "${record}" in table "${table}" in db "${database}".`));
const result = await Result.findByPk(record);
if (result) {
const res = result.get({ plain: true });
// column name is result
const parseRes = safeJSONParse(res?.result);
if (!parseRes.ok) return ctx.fail(new DbError(parseRes.error));
return ctx.pass(parseRes.parsed);
} else {
return ctx.fail(new DbError(`Returned null for id "${record}" in table "${table}"`));
}
db.close();
} catch (e) {
return ctx.fail(new DbError(`[DATABASE] Database error: ${e}`));
return ctx.fail(new DbError(e.message));
} finally {
db.close();
}
return ctx.pass(output);
},
}
);

/**
Expand All @@ -157,9 +174,11 @@ export const saveVar = p.new('connect',
const varName = ctx.fetch('name') as string;
const database = ctx.fetchConnect()[0] as string;
const table = ctx.fetch('table') as string;
const initRes = safeDbInit(database)
if (!initRes.ok) return ctx.fail(new DbError(initRes.error));
const db = initRes.db;

try {
const db = new Sequelize(database, { logging: false });
Result.init(
{ result: DataTypes.TEXT },
{
Expand All @@ -169,23 +188,19 @@ export const saveVar = p.new('connect',
}
);
await Result.sync();
try {
// column name must be result
await Result.create({
result: JSON.stringify({
[varName]: varObj,
}),
});
} catch (e) {
return ctx.fail(new DbError(`[DATABASE]
Error in table "${table}" in db "${database}": ${e}`));
}
db.close();
// column name must be result
await Result.create({
result: JSON.stringify({
[varName]: varObj,
}),
});
return ctx.pass(null);
} catch (e) {
return ctx.fail(new DbError(`[DATABASE] Database error: ${e}`));
return ctx.fail(new DbError(e.message));
} finally {
db.close();
}
return ctx.pass(null);
},
}
);

export const db = p;
Loading

0 comments on commit 2c76af0

Please sign in to comment.