From ea3d7388234f23c4a5bc7ceb55c493a097b76c12 Mon Sep 17 00:00:00 2001 From: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Date: Wed, 2 Nov 2022 19:58:02 +0100 Subject: [PATCH] Feat(medusa): config error handling in loaders (#2514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **What** - add error handling when loading project config **How** - Add error parameter to get-medusa-config result if an error was thrown (previously we returned an empty config) - Discussion: A different, but equally valid approach could be just throwing the error rather than creating an error parameter. This causes a more ugly output without warnings and changes the api a bit but it would force error handling. wdyt? **Why** - cli would fail with database error `databaseMissingDriverError` if config was invalid, ex. missing a comma ### example (missing `,` in config) **old output** ``` Successfully compiled 2 files with Babel (143ms). [medusa-config] ⚠️ redis_url not found. A fake redis instance will be used. [medusa-config] ⚠️ database_type not found. fallback to default sqlite. info: Using flag MEDUSA_FF_ORDER_EDITING from environment with value true info: Using flag MEDUSA_FF_SALES_CHANNELS from environment with value true info: Using flag MEDUSA_FF_TAX_INCLUSIVE_PRICING from environment with value true info: Using fake Redis ✔ Models initialized – 13ms ✔ Plugin models initialized – 0ms ✔ Repositories initialized – 17ms ⠋ Initializing databaseMissingDriverError: Wrong driver: "undefined" given. Supported drivers are: "aurora-data-api", "aurora-data-api-pg", "better-sqlite3", "capacitor", "cockroachdb", "cordova", "expo", "mariadb", "mongodb", "mssql", "mysql", "nativescript", "oracle", "postgres", "react-native", "sap", "sqlite", "sqljs". ``` **new output** ``` Successfully compiled 2 files with Babel (185ms). error: Error in loading config: Unexpected identifier error: /Users/phko/projects/community/my-medusa-store/medusa-config.js:129 plugins, ^^^^^^^ SyntaxError: Unexpected identifier at compileFunction () at Object.compileFunction (node:vm:352:18) at wrapSafe (node:internal/modules/cjs/loader:1033:15) at Module._compile (node:internal/modules/cjs/loader:1069:27) at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10) at Module.load (node:internal/modules/cjs/loader:981:32) at Function.Module._load (node:internal/modules/cjs/loader:822:12) at Module.require (node:internal/modules/cjs/loader:1005:19) at require (node:internal/modules/cjs/helpers:102:18) at getConfigFile (/Users/phko/projects/community/my-medusa-store/node_modules/medusa-core-utils/dist/get-config-file.js:26:20) ``` --- .changeset/fresh-dingos-fix.md | 6 ++++ .../api/__tests__/batch-jobs/api.js | 1 - .../medusa-core-utils/src/get-config-file.js | 7 ++-- packages/medusa/src/commands/migrate.js | 7 +++- packages/medusa/src/commands/seed.js | 7 +++- .../src/commands/utils/get-migrations.js | 9 +++++- packages/medusa/src/loaders/config.ts | 19 ++++++++++- packages/medusa/src/loaders/index.ts | 5 ++- packages/medusa/src/utils/db-aware-column.ts | 32 +++++++------------ .../medusa/src/utils/manual-auto-increment.ts | 17 ++++------ 10 files changed, 71 insertions(+), 39 deletions(-) create mode 100644 .changeset/fresh-dingos-fix.md diff --git a/.changeset/fresh-dingos-fix.md b/.changeset/fresh-dingos-fix.md new file mode 100644 index 0000000000..8e34ec7f7a --- /dev/null +++ b/.changeset/fresh-dingos-fix.md @@ -0,0 +1,6 @@ +--- +"medusa-core-utils": patch +"@medusajs/medusa": patch +--- + +Feat(medusa): invalid medusa-config handling in loaders diff --git a/integration-tests/api/__tests__/batch-jobs/api.js b/integration-tests/api/__tests__/batch-jobs/api.js index 23a04f5092..d9c247d686 100644 --- a/integration-tests/api/__tests__/batch-jobs/api.js +++ b/integration-tests/api/__tests__/batch-jobs/api.js @@ -306,7 +306,6 @@ describe("/admin/batch-jobs", () => { await api .post(`/admin/batch-jobs/${jobId}/cancel`, {}, adminReqConfig) .catch((err) => { - console.log(err) expect(err.response.status).toEqual(400) expect(err.response.data.type).toEqual("not_allowed") expect(err.response.data.message).toEqual( diff --git a/packages/medusa-core-utils/src/get-config-file.js b/packages/medusa-core-utils/src/get-config-file.js index 6e8d731db2..e6b909b893 100644 --- a/packages/medusa-core-utils/src/get-config-file.js +++ b/packages/medusa-core-utils/src/get-config-file.js @@ -4,17 +4,20 @@ import path from "path" * Attempts to resolve the config file in a given root directory. * @param {string} rootDir - the directory to find the config file in. * @param {string} configName - the name of the config file. - * @return {object} an object containing the config module and its path. + * @return {object} an object containing the config module and its path as well as an error property if the config couldn't be loaded. */ function getConfigFile(rootDir, configName) { const configPath = path.join(rootDir, configName) let configFilePath = `` let configModule + try { configFilePath = require.resolve(configPath) configModule = require(configFilePath) } catch (err) { - return {} + return { + error: err, + } } return { configModule, configFilePath } diff --git a/packages/medusa/src/commands/migrate.js b/packages/medusa/src/commands/migrate.js index 6800571223..16fd8575d7 100644 --- a/packages/medusa/src/commands/migrate.js +++ b/packages/medusa/src/commands/migrate.js @@ -1,6 +1,7 @@ import { createConnection } from "typeorm" import { getConfigFile } from "medusa-core-utils" import featureFlagLoader from "../loaders/feature-flags" +import { handleConfigError } from "../loaders/config" import Logger from "../loaders/logger" import getMigrations from "./utils/get-migrations" @@ -11,7 +12,11 @@ const t = async function ({ directory }) { args.shift() args.shift() - const { configModule } = getConfigFile(directory, `medusa-config`) + const { configModule, error } = getConfigFile(directory, `medusa-config`) + + if (error) { + handleConfigError(error) + } const featureFlagRouter = featureFlagLoader(configModule) diff --git a/packages/medusa/src/commands/seed.js b/packages/medusa/src/commands/seed.js index 1a372f0e35..424ce9959c 100644 --- a/packages/medusa/src/commands/seed.js +++ b/packages/medusa/src/commands/seed.js @@ -6,6 +6,7 @@ import { sync as existsSync } from "fs-exists-cached" import { getConfigFile } from "medusa-core-utils" import { track } from "medusa-telemetry" +import { handleConfigError } from "../loaders/config" import Logger from "../loaders/logger" import loaders from "../loaders" @@ -29,7 +30,11 @@ const t = async function ({ directory, migrate, seedFile }) { } } - const { configModule } = getConfigFile(directory, `medusa-config`) + const { configModule, error } = getConfigFile(directory, `medusa-config`) + + if (error) { + handleConfigError(error) + } const featureFlagRouter = featureFlagLoader(configModule) diff --git a/packages/medusa/src/commands/utils/get-migrations.js b/packages/medusa/src/commands/utils/get-migrations.js index eb4afb91f7..d0148f6d30 100644 --- a/packages/medusa/src/commands/utils/get-migrations.js +++ b/packages/medusa/src/commands/utils/get-migrations.js @@ -4,6 +4,8 @@ import fs from "fs" import { isString } from "lodash" import { sync as existsSync } from "fs-exists-cached" import { getConfigFile, createRequireFromPath } from "medusa-core-utils" +import { handleConfigError } from "../../loaders/config" +import logger from "../../loaders/logger" function createFileContentHash(path, files) { return path + files @@ -88,7 +90,12 @@ function resolvePlugin(pluginName) { } export default async (directory, featureFlagRouter) => { - const { configModule } = getConfigFile(directory, `medusa-config`) + const { configModule, error } = getConfigFile(directory, `medusa-config`) + + if (error) { + handleConfigError(error) + } + const { plugins } = configModule const resolved = plugins.map((plugin) => { diff --git a/packages/medusa/src/loaders/config.ts b/packages/medusa/src/loaders/config.ts index 13c8be0ada..015ebd5532 100644 --- a/packages/medusa/src/loaders/config.ts +++ b/packages/medusa/src/loaders/config.ts @@ -1,5 +1,6 @@ import { ConfigModule } from "../types/global" import { getConfigFile } from "medusa-core-utils/dist" +import logger from "./logger" const isProduction = ["production", "prod"].includes(process.env.NODE_ENV || "") @@ -9,9 +10,25 @@ const errorHandler = isProduction } : console.log +export const handleConfigError = (error: Error): void => { + logger.error(`Error in loading config: ${error.message}`) + if (error.stack) { + logger.error(error.stack) + } + process.exit(1) +} + export default (rootDirectory: string): ConfigModule => { - const { configModule } = getConfigFile(rootDirectory, `medusa-config`) as { + const { configModule, error } = getConfigFile( + rootDirectory, + `medusa-config` + ) as { configModule: ConfigModule + error: Error | null + } + + if (error) { + handleConfigError(error) } if (!configModule?.projectConfig?.redis_url) { diff --git a/packages/medusa/src/loaders/index.ts b/packages/medusa/src/loaders/index.ts index 35d0bd2389..e3bac0b29d 100644 --- a/packages/medusa/src/loaders/index.ts +++ b/packages/medusa/src/loaders/index.ts @@ -115,7 +115,10 @@ export default async ({ const dbActivity = Logger.activity("Initializing database") track("DATABASE_INIT_STARTED") - const dbConnection = await databaseLoader({ container, configModule }) + const dbConnection = await databaseLoader({ + container, + configModule, + }) const dbAct = Logger.success(dbActivity, "Database initialized") || {} track("DATABASE_INIT_COMPLETED", { duration: dbAct.duration }) diff --git a/packages/medusa/src/utils/db-aware-column.ts b/packages/medusa/src/utils/db-aware-column.ts index 993e359061..d65d6324d6 100644 --- a/packages/medusa/src/utils/db-aware-column.ts +++ b/packages/medusa/src/utils/db-aware-column.ts @@ -18,16 +18,12 @@ const pgSqliteGenerationMapping: { let dbType: string export function resolveDbType(pgSqlType: ColumnType): ColumnType { if (!dbType) { - try { - const { configModule } = getConfigFile( - path.resolve("."), - `medusa-config` - ) as any - dbType = configModule.projectConfig.database_type - } catch (error) { - // Default to Postgres to allow for e.g. migrations to run - dbType = "postgres" - } + const { configModule } = getConfigFile( + path.resolve("."), + `medusa-config` + ) as any + + dbType = configModule?.projectConfig?.database_type || "postgres" } if (dbType === "sqlite" && (pgSqlType as string) in pgSqliteTypeMapping) { @@ -40,16 +36,12 @@ export function resolveDbGenerationStrategy( pgSqlType: "increment" | "uuid" | "rowid" ): "increment" | "uuid" | "rowid" { if (!dbType) { - try { - const { configModule } = getConfigFile( - path.resolve("."), - `medusa-config` - ) as any - dbType = configModule.projectConfig.database_type - } catch (error) { - // Default to Postgres to allow for e.g. migrations to run - dbType = "postgres" - } + const { configModule } = getConfigFile( + path.resolve("."), + `medusa-config` + ) as any + + dbType = configModule?.projectConfig?.database_type || "postgres" } if (dbType === "sqlite" && pgSqlType in pgSqliteTypeMapping) { diff --git a/packages/medusa/src/utils/manual-auto-increment.ts b/packages/medusa/src/utils/manual-auto-increment.ts index 2ab0b82d15..7f753ab747 100644 --- a/packages/medusa/src/utils/manual-auto-increment.ts +++ b/packages/medusa/src/utils/manual-auto-increment.ts @@ -5,17 +5,12 @@ import { getConnection } from "typeorm" export async function manualAutoIncrement( tableName: string ): Promise { - let dbType - try { - const { configModule } = getConfigFile( - path.resolve("."), - `medusa-config` - ) as any - dbType = configModule.projectConfig.database_type - } catch (error) { - // Default to Postgres to allow for e.g. migrations to run - dbType = "postgres" - } + const { configModule } = getConfigFile( + path.resolve("."), + `medusa-config` + ) as any + + const dbType = configModule?.projectConfig?.database_type || "postgres" if (dbType === "sqlite") { const connection = getConnection()