From 2c76af0d90edcfece8a8811dad0100fd55fbba9b Mon Sep 17 00:00:00 2001 From: Matteo Cristino <102997993+matteo-cristino@users.noreply.github.com> Date: Mon, 1 Jul 2024 09:57:51 +0200 Subject: [PATCH] refactor(db): improve error reporting and code clean up (#171) * 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 --- pkg/db/package.json | 1 + pkg/db/src/plugin.ts | 163 +++++++++++---------- pkg/db/test/e2e.ts | 297 ++++++++++++++++----------------------- pkg/db/test/raw_query.ts | 3 +- pnpm-lock.yaml | 3 + 5 files changed, 215 insertions(+), 252 deletions(-) diff --git a/pkg/db/package.json b/pkg/db/package.json index aff2a588..3dc37f27 100644 --- a/pkg/db/package.json +++ b/pkg/db/package.json @@ -3,6 +3,7 @@ "version": "1.33.13", "dependencies": { "@slangroom/core": "workspace:*", + "@slangroom/shared": "workspace:*", "sequelize": "^6.16.0", "sqlite3": "^5.0.0" }, diff --git a/pkg/db/src/plugin.ts b/pkg/db/src/plugin.ts index c1aef50c..fd972815 100644 --- a/pkg/db/src/plugin.ts +++ b/pkg/db/src/plugin.ts @@ -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 { @@ -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 }> => { + 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(); } } @@ -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); + } ); /** @@ -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); + } ); /** @@ -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 }, { @@ -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); - }, + } ); /** @@ -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 }, { @@ -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; diff --git a/pkg/db/test/e2e.ts b/pkg/db/test/e2e.ts index 581c2589..0c086071 100644 --- a/pkg/db/test/e2e.ts +++ b/pkg/db/test/e2e.ts @@ -11,42 +11,40 @@ import fs from "fs"; import packageJson from '@slangroom/db/package.json' with { type: 'json' }; process.env['FILES_DIR'] = "./test"; -const dbPath1 = "sqlite://./test/db1.db"; -const dbPath2 = "sqlite://./test/db2.db"; +const fileDb1 = './test/db1.db'; +const fileDb2 = './test/db2.db'; +const dbPath1 = `sqlite://${fileDb1}`; +const dbPath2 = `sqlite://${fileDb2}`; + +class Result1 extends Model { + public result!: string; +} const stripAnsiCodes = (str: string) => str.replace(/[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g, ''); const complexQuery = `Rule unknown ignore - Scenario 'ecdh': Create the keypair - # the value of the record could be 0 to max could be - Given I connect to 'myDb1' and send record 'n' and send table 'myTable' and read the record of the table and output into 'myZenroomStringDictionary' - Given I am 'John' - Given I have a 'string' named 'key_name' - Given I have a 'string' named 'myDb1' - Given I have a 'string' named 'myDb2' - Given I have a 'string' named 'myTable' - Given I have a 'string' named 'myCache' - Given I have a 'string dictionary' named 'myZenroomStringDictionary' - When I create the ecdh key - When I create the signature of 'myZenroomStringDictionary' - When I create the array of '8' random objects of '256' bits - Then print all data - Then print the keyring - Then I connect to 'myDb2' and send variable 'keyring' and send name 'key_name' and send table 'myCache' and save the variable in the database table - `; - -test.afterEach(() => { - try { - fs.unlinkSync("./test/db1.db"); - } catch (e) { - throw e; - } - try { - fs.unlinkSync("./test/db2.db"); - } catch (e) { - throw e; - } -}); +Scenario 'ecdh': Create the keypair +# the value of the record could be 0 to max could be +Given I connect to 'myDb1' and send record 'n' and send table 'myTable' and read the record of the table and output into 'myZenroomStringDictionary' +Given I am 'John' +Given I have a 'string' named 'key_name' +Given I have a 'string' named 'myDb1' +Given I have a 'string' named 'myDb2' +Given I have a 'string' named 'myTable' +Given I have a 'string' named 'myCache' +Given I have a 'string dictionary' named 'myZenroomStringDictionary' +When I create the ecdh key +When I create the signature of 'myZenroomStringDictionary' +When I create the array of '8' random objects of '256' bits +Then print all data +Then print the keyring +Then I connect to 'myDb2' and send variable 'keyring' and send name 'key_name' and send table 'myCache' and save the variable in the database table`; + +const readQuery = `Rule unknown ignore +Given I connect to 'db' and send record 'n' and send table 'myTable' and read the record of the table and output into 'result' +Given I have a 'string dictionary' named 'result' +Then print all data`; +// test.beforeEach(async () => { const sequelize = new Sequelize(dbPath1, { logging: false }); @@ -58,25 +56,25 @@ test.beforeEach(async () => { freezeTableName: true, }); await Table.sync(); - try { - const results = Array(5).fill(JSON.stringify({ - testkey: "test value" - })); - for (const result of results) { - await Table.create({ - result - }); - } - } catch (e) { - throw e; + const results = Array(5).fill(JSON.stringify({ + testkey: "test value" + })); + for (const result of results) { + await Table.create({ + result + }); } sequelize.close(); }); +test.afterEach(() => { + if (fs.existsSync(fileDb1)) fs.unlinkSync(fileDb1); + if (fs.existsSync(fileDb2)) fs.unlinkSync(fileDb2); +}); -test.serial('Middleware db should work and response includes variable for db', async (t) => { +test.serial('Db read from a db and write in another one', async (t) => { const slangroom = new Slangroom(db); - const result = slangroom.execute(complexQuery, { + const res = await slangroom.execute(complexQuery, { data: { "n": 3, "key_name": "keyring", @@ -92,119 +90,88 @@ test.serial('Middleware db should work and response includes variable for db', a } }, }); - const res = await result; + // check the result + t.true( + Object.keys(res.result).includes("keyring"), + 'could not find "keyring" in response' + ); + t.true( + Object.keys(res.result).includes("myZenroomStringDictionary"), + 'could not find "myZenroomStringDictionary" in response' + ); + // check the database + const sequelize2 = new Sequelize(dbPath2, { logging: false }); + const Result2 = sequelize2.define( + "firstCache", { + result: { + type: DataTypes.STRING, + }, + }, { + freezeTableName: true, + } + ); + await Result2.sync(); + let res2; try { - t.true( - Object.keys(res.result).includes("keyring"), "" - ); - t.true( - Object.keys(res.result).includes("myZenroomStringDictionary"), - 'could not find "myZenroomStringDictionary" in response' - ); + let query = await Result2.findByPk(1) as Result1; + query = query?.get({ plain: true }); + res2 = JSON.parse(query?.["result"] ?? "{}"); } catch (e) { - throw e; + res2 = null; } + sequelize2.close(); + t.deepEqual( + res.result['keypair'], + res2.keypair, + "The value stored in the database should be the same as the value in the response body" + ); }); -class Result1 extends Model { - public result!: any; -} - -test.serial( - "Middleware db should save the result of variable given in zencode to db2", - async (t) => { - try { - const slangroom = new Slangroom(db); - const result = slangroom.execute(complexQuery, { - data: { - "n": 3, - "key_name": "keyring", - "myDb1": "sqlite://./test/db1.db", - "myDb2": "sqlite://./test/db2.db", - "myTable": "firstTable", - "myCache": "firstCache", - "myOtherZenroomStringDictionary": { - "data": { - "data1": "9WgBlK+Kcq3AKpmhituXQe4UPkzH3zpZiQa4Szm1Q40=", - "data2": "BCEo8MgRiSxtLfxE4UEDVnbdZ21h+xc+egLIRk3VTagpJxlBfu9MjqXGUi2N7tIewpcDrr5V7Z2cmMcNsbKWSGQ=" - } - } - }, - }); - const res = await result; - const sequelize2 = new Sequelize(dbPath2, { logging: false }); - const Result2 = sequelize2.define( - "firstCache", { - result: { - type: DataTypes.STRING, - }, - }, { - freezeTableName: true, - } - ); - await Result2.sync(); - let ress; - try { - let query = await Result2.findByPk(1) as Result1; - query = query!.get({ - plain: true - }); - ress = JSON.parse(query!["result"]); - } catch (e) { - ress = null; - } - sequelize2.close(); - t.deepEqual( - res.result['keypair'], - ress.keypair, - "The value stored in the database should be the same as the value in the response body" - ); - } catch (e) { - throw e; - } - } -); - test.serial('Db should fail for wrong record', async (t) => { - const failQuery = `Rule unknown ignore - Scenario 'ecdh': Create the keypair - Given I connect to 'myDb2' and send variable 'obj_1' and send name 'var_name' and send table 'myCache' and save the variable in the database table - # the value of the record could be 0 to max could be - Given I connect to 'myDb1' and send record 'n' and send table 'myTable' and read the record of the table and output into 'myZenroomStringDictionary' - Given I am 'John' - Given I have a 'string' named 'myDb1' - Given I have a 'string' named 'myDb2' - Given I have a 'string' named 'myTable' - Given I have a 'string' named 'myCache' - Given I have a 'string dictionary' named 'myZenroomStringDictionary' - Then print all data - `; const slangroom = new Slangroom(db); - const result = slangroom.execute(failQuery, { + const result = slangroom.execute(readQuery, { data: { "n": 30, - "var_name": "obj_1", - "obj_1": "pippo", - "myDb1": "sqlite://./test/db1.db", - "myDb2": "sqlite://./test/db2.db", + "db": "sqlite://./test/db1.db", "myTable": "firstTable", - "myCache": "firstCache", - "myOtherZenroomStringDictionary": { - "data": { - "data1": "9WgBlK+Kcq3AKpmhituXQe4UPkzH3zpZiQa4Szm1Q40=", - "data2": "BCEo8MgRiSxtLfxE4UEDVnbdZ21h+xc+egLIRk3VTagpJxlBfu9MjqXGUi2N7tIewpcDrr5V7Z2cmMcNsbKWSGQ=" - } - } + } + }); + + const error = await t.throwsAsync(result); + t.is(stripAnsiCodes((error as Error).message), + `0 | Rule unknown ignore +1 | Given I connect to 'db' and send record 'n' and send table 'myTable' and read the record of the table and output into 'result' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +2 | Given I have a 'string dictionary' named 'result' +3 | Then print all data + +Error colors: + - error + - suggested words + - missing words + - extra words + +Slangroom @slangroom/db@${packageJson.version} Error: Returned null for id "30" in table "firstTable" +`); +}); + +test.serial('Db should fail for wrong db dialect', async (t) => { + const slangroom = new Slangroom(db); + const result = slangroom.execute(readQuery, { + data: { + "n": 3, + "myTable": "firstTable", + "db": "mariodb://usr:pwd@example.com" }, }); const error = await t.throwsAsync(result); t.is(stripAnsiCodes((error as Error).message), - `3 | # the value of the record could be 0 to max could be -4 | Given I connect to 'myDb1' and send record 'n' and send table 'myTable' and read the record of the table and output into 'myZenroomStringDictionary' - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -5 | Given I am 'John' -6 | Given I have a 'string' named 'myDb1' + `0 | Rule unknown ignore +1 | Given I connect to 'db' and send record 'n' and send table 'myTable' and read the record of the table and output into 'result' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +2 | Given I have a 'string dictionary' named 'result' +3 | Then print all data Error colors: - error @@ -212,51 +179,27 @@ Error colors: - missing words - extra words -Slangroom @slangroom/db@${packageJson.version} Error: [DATABASE] - Returned null for id "30" in table "firstTable" in db "sqlite://./test/db1.db". +Slangroom @slangroom/db@${packageJson.version} Error: The dialect mariodb is not supported. Supported dialects: mssql, mariadb, mysql, oracle, postgres, db2 and sqlite. `); }); -test.serial('Db should fail for wrong db', async (t) => { - const failQuery = `Rule unknown ignore - Scenario 'ecdh': Create the keypair - # the value of the record could be 0 to max could be - Given I connect to 'myDb1' and send record 'n' and send table 'myTable' and read the record of the table and output into 'myZenroomStringDictionary' - Given I am 'John' - Given I have a 'string' named 'obj_1' - Given I have a 'string' named 'var_name' - Given I have a 'string' named 'myDb1' - Given I have a 'string' named 'myDb2' - Given I have a 'string' named 'myTable' - Given I have a 'string' named 'myCache' - Given I have a 'string dictionary' named 'myZenroomStringDictionary' - Then print all data - Then I connect to 'myDb2' and send variable 'obj_1' and send name 'var_name' and send table 'myCache' and save the variable in the database table - Then I connect to 'myTable' and send variable 'obj_1' and send name 'var_name' and send table 'myCache' and save the variable in the database table`; +test.serial('Db should fail with wrong db url', async (t) => { const slangroom = new Slangroom(db); - const result = slangroom.execute(failQuery, { + const result = slangroom.execute(readQuery, { data: { "n": 3, - "var_name": "obj_1", - "obj_1": "pippo", - "myDb1": "sqlite://./test/db1.db", - "myDb2": "sqlite://./test/db2.db", "myTable": "firstTable", - "myCache": "firstCache", - "myOtherZenroomStringDictionary": { - "data": { - "data1": "9WgBlK+Kcq3AKpmhituXQe4UPkzH3zpZiQa4Szm1Q40=", - "data2": "BCEo8MgRiSxtLfxE4UEDVnbdZ21h+xc+egLIRk3VTagpJxlBfu9MjqXGUi2N7tIewpcDrr5V7Z2cmMcNsbKWSGQ=" - } - } + "db": "mariadb/user:pwdo@example.com" }, }); const error = await t.throwsAsync(result); t.is(stripAnsiCodes((error as Error).message), - `13 | Then I connect to 'myDb2' and send variable 'obj_1' and send name 'var_name' and send table 'myCache' and save the variable in the database table -14 | Then I connect to 'myTable' and send variable 'obj_1' and send name 'var_name' and send table 'myCache' and save the variable in the database table - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + `0 | Rule unknown ignore +1 | Given I connect to 'db' and send record 'n' and send table 'myTable' and read the record of the table and output into 'result' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +2 | Given I have a 'string dictionary' named 'result' +3 | Then print all data Error colors: - error @@ -264,6 +207,6 @@ Error colors: - missing words - extra words -Slangroom @slangroom/db@${packageJson.version} Error: [DATABASE] Database error: TypeError: Cannot read properties of null (reading 'replace') +Slangroom @slangroom/db@${packageJson.version} Error: Invalid URL `); }); diff --git a/pkg/db/test/raw_query.ts b/pkg/db/test/raw_query.ts index bd9adfb3..da9cf863 100644 --- a/pkg/db/test/raw_query.ts +++ b/pkg/db/test/raw_query.ts @@ -4,6 +4,7 @@ import test from "ava"; import { Slangroom } from '@slangroom/core'; +import type { JsonableArray } from '@slangroom/shared'; import { db } from '@slangroom/db'; import sqlite3 from "sqlite3"; // read the version from the package.json @@ -70,7 +71,7 @@ test('Db should execute raw queries', async (t) => { t.true(lastID_2 && lastID_2 > 1); const res_3 = res.result['result_3'] as string[]; t.deepEqual(res_3, []); - const res_4 = res.result['result_4'] as Record[]; + const res_4 = res.result['result_4'] as JsonableArray; const elem = typeof (res_4) !== 'undefined' ? res_4[0] || {} : {}; t.deepEqual(Object.keys(elem), ['date', 'name']); const res_5 = res.result['result_5'] as { changes?: string, lastID?: string }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5ffd012a..e34d9415 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -159,6 +159,9 @@ importers: '@slangroom/core': specifier: workspace:* version: link:../core + '@slangroom/shared': + specifier: workspace:* + version: link:../shared sequelize: specifier: ^6.16.0 version: 6.37.3(sqlite3@5.1.7)