From 82a176e30e47a7d11caaf31c3023bd8db588b465 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Tue, 2 Apr 2024 16:10:17 +0200 Subject: [PATCH] chore(medusa-test-utils):Handle errors gracefully (#6901) **What** - Better error handling and error message - update deps management and dynamic import/require - Pass a new flag to the modules loaders for the module loaders to be able to act depending on it. In that case, the module can determine what should be run or not. e.g in the workflow engine redis, when we are only partially loading the module, we do not want to set the Distributed transaction storage --- .changeset/khaki-birds-tickle.md | 9 ++ .../environment-helpers/bootstrap-app.js | 30 ++--- packages/medusa-test-utils/package.json | 2 +- .../medusa-test-utils/src/init-modules.ts | 20 +-- .../src/medusa-test-runner.ts | 117 ++++++++++++------ .../src/module-test-runner.ts | 19 +-- .../src/loaders/utils/load-internal.ts | 1 + packages/modules-sdk/src/medusa-module.ts | 4 +- packages/types/src/modules-sdk/index.ts | 1 + .../src/loaders/redis.ts | 2 + .../src/loaders/utils.ts | 5 +- .../src/services/workflow-orchestrator.ts | 7 +- yarn.lock | 2 +- 13 files changed, 141 insertions(+), 78 deletions(-) create mode 100644 .changeset/khaki-birds-tickle.md diff --git a/.changeset/khaki-birds-tickle.md b/.changeset/khaki-birds-tickle.md new file mode 100644 index 0000000000..9258032fdd --- /dev/null +++ b/.changeset/khaki-birds-tickle.md @@ -0,0 +1,9 @@ +--- +"@medusajs/medusa": patch +"medusa-test-utils": patch +"@medusajs/modules-sdk": patch +"@medusajs/types": patch +"@medusajs/workflow-engine-redis": patch +--- + +chore(medusa-test-utils): Handle errors gracefully + Do not set Distributed storage on partial loading diff --git a/integration-tests/environment-helpers/bootstrap-app.js b/integration-tests/environment-helpers/bootstrap-app.js index 6fc21793fc..035dd02a94 100644 --- a/integration-tests/environment-helpers/bootstrap-app.js +++ b/integration-tests/environment-helpers/bootstrap-app.js @@ -14,20 +14,17 @@ async function bootstrapApp({ cwd, env = {} } = {}) { const loaders = require("@medusajs/medusa/dist/loaders").default - const { container, dbConnection, pgConnection, disposeResources } = - await loaders({ - directory: path.resolve(cwd || process.cwd()), - expressApp: app, - isTest: false, - }) + const { container, shutdown } = await loaders({ + directory: path.resolve(cwd || process.cwd()), + expressApp: app, + isTest: false, + }) const PORT = await getPort() return { - disposeResources, + shutdown, container, - db: dbConnection, - pgConnection, app, port: PORT, } @@ -40,7 +37,12 @@ module.exports = { env = {}, skipExpressListen = false, } = {}) => { - const { app, port, container, db, pgConnection } = await bootstrapApp({ + const { + app, + port, + container, + shutdown: medusaShutdown, + } = await bootstrapApp({ cwd, env, }) @@ -53,13 +55,7 @@ module.exports = { } const shutdown = async () => { - await Promise.all([ - container.dispose(), - expressServer.close(), - db?.destroy(), - pgConnection?.context?.destroy(), - container.dispose(), - ]) + await Promise.all([expressServer.close(), medusaShutdown()]) if (typeof global !== "undefined" && global?.gc) { global.gc() diff --git a/packages/medusa-test-utils/package.json b/packages/medusa-test-utils/package.json index fdc7849bb6..b094534024 100644 --- a/packages/medusa-test-utils/package.json +++ b/packages/medusa-test-utils/package.json @@ -32,6 +32,7 @@ }, "peerDependencies": { "@medusajs/medusa": ">1.19", + "@medusajs/modules-sdk": "^1.12.10", "axios": "^0.28.0", "express": "^4.18.3", "get-port": "^5.1.0", @@ -59,7 +60,6 @@ } }, "dependencies": { - "@medusajs/modules-sdk": "^1.12.10", "@medusajs/utils": "^1.11.8", "@mikro-orm/migrations": "5.9.7", "@mikro-orm/postgresql": "5.9.7", diff --git a/packages/medusa-test-utils/src/init-modules.ts b/packages/medusa-test-utils/src/init-modules.ts index e7f0e73756..029635231b 100644 --- a/packages/medusa-test-utils/src/init-modules.ts +++ b/packages/medusa-test-utils/src/init-modules.ts @@ -1,9 +1,8 @@ import { - MedusaApp, - MedusaModule, - MedusaModuleConfig, + ExternalModuleDeclaration, + InternalModuleDeclaration, ModuleJoinerConfig, -} from "@medusajs/modules-sdk" +} from "@medusajs/types" import { ContainerRegistrationKeys, ModulesSdkUtils, @@ -16,7 +15,12 @@ export interface InitModulesOptions { clientUrl: string schema?: string } - modulesConfig: MedusaModuleConfig + modulesConfig: { + [key: string]: + | string + | boolean + | Partial + } joinerConfig?: ModuleJoinerConfig[] preventConnectionDestroyWarning?: boolean } @@ -28,6 +32,8 @@ export async function initModules({ joinerConfig, preventConnectionDestroyWarning = false, }: InitModulesOptions) { + const moduleSdkImports = require("@medusajs/modules-sdk") + injectedDependencies ??= {} let sharedPgConnection = @@ -44,7 +50,7 @@ export async function initModules({ sharedPgConnection } - const medusaApp = await MedusaApp({ + const medusaApp = await moduleSdkImports.MedusaApp({ modulesConfig, servicesConfig: joinerConfig, injectedDependencies, @@ -64,7 +70,7 @@ export async function initModules({ ) } } - MedusaModule.clearInstances() + moduleSdkImports.MedusaModule.clearInstances() } return { diff --git a/packages/medusa-test-utils/src/medusa-test-runner.ts b/packages/medusa-test-utils/src/medusa-test-runner.ts index 5c221e64d3..38e660b65f 100644 --- a/packages/medusa-test-utils/src/medusa-test-runner.ts +++ b/packages/medusa-test-utils/src/medusa-test-runner.ts @@ -34,6 +34,10 @@ const dbTestUtilFactory = (): any => ({ schema, }: { forceDelete?: string[]; schema?: string } = {}) { forceDelete ??= [] + if (!this.db_) { + return + } + const manager = this.db_.manager schema ??= "public" @@ -90,9 +94,7 @@ export function medusaIntegrationTestRunner({ schema?: string debug?: boolean force_modules_migration?: boolean - testSuite: ( - options: MedusaSuiteOptions - ) => () => void + testSuite: (options: MedusaSuiteOptions) => void }) { const tempName = parseInt(process.env.JEST_WORKER_ID || "1") moduleName = moduleName ?? Math.random().toString(36).substring(7) @@ -147,37 +149,63 @@ export function medusaIntegrationTestRunner({ const beforeAll_ = async () => { await dbUtils.create(dbName) - const { dbDataSource, pgConnection } = await initDb({ - cwd, - env, - force_modules_migration, - database_extra: {}, - dbUrl: dbConfig.clientUrl, - dbSchema: dbConfig.schema, - }) - dbUtils.db_ = dbDataSource - dbUtils.pgConnection_ = pgConnection - const { - shutdown: serverShutdown, - container: container_, - port, - } = await startBootstrapApp({ - cwd, - env, - }) + let dataSourceRes + let pgConnectionRes + + try { + const { dbDataSource, pgConnection } = await initDb({ + cwd, + env, + force_modules_migration, + database_extra: {}, + dbUrl: dbConfig.clientUrl, + dbSchema: dbConfig.schema, + }) + + dataSourceRes = dbDataSource + pgConnectionRes = pgConnection + } catch (error) { + console.error("Error initializing database", error?.message) + throw error + } + + dbUtils.db_ = dataSourceRes + dbUtils.pgConnection_ = pgConnectionRes + + let containerRes + let serverShutdownRes + let portRes + try { + const { + shutdown = () => void 0, + container, + port, + } = await startBootstrapApp({ + cwd, + env, + }) + + containerRes = container + serverShutdownRes = shutdown + portRes = port + } catch (error) { + console.error("Error starting the app", error?.message) + throw error + } const cancelTokenSource = axios.CancelToken.source() - apiUtils = axios.create({ - baseURL: `http://localhost:${port}`, - cancelToken: cancelTokenSource.token, - }) - container = container_ + container = containerRes shutdown = async () => { - await serverShutdown() + await serverShutdownRes() cancelTokenSource.cancel("Request canceled by shutdown") } + + apiUtils = axios.create({ + baseURL: `http://localhost:${portRes}`, + cancelToken: cancelTokenSource.token, + }) } const beforeEach_ = async () => { @@ -191,26 +219,37 @@ export function medusaIntegrationTestRunner({ const copiedContainer = createMedusaContainer({}, container) if (process.env.MEDUSA_FF_MEDUSA_V2 != "true") { - const defaultLoader = - require("@medusajs/medusa/dist/loaders/defaults").default - await defaultLoader({ - container: copiedContainer, - }) + try { + const defaultLoader = + require("@medusajs/medusa/dist/loaders/defaults").default + await defaultLoader({ + container: copiedContainer, + }) + } catch (error) { + console.error("Error runner medusa loaders", error?.message) + throw error + } } - const medusaAppLoaderRunner = - require("@medusajs/medusa/dist/loaders/medusa-app").runModulesLoader - await medusaAppLoaderRunner({ - container: copiedContainer, - configModule: container.resolve("configModule"), - }) + try { + const medusaAppLoaderRunner = + require("@medusajs/medusa/dist/loaders/medusa-app").runModulesLoader + await medusaAppLoaderRunner({ + container: copiedContainer, + configModule: container.resolve("configModule"), + }) + } catch (error) { + console.error("Error runner modules loaders", error?.message) + throw error + } } const afterEach_ = async () => { try { await dbUtils.teardown({ schema }) } catch (error) { - console.error("Error tearing down database:", error) + console.error("Error tearing down database:", error?.message) + throw error } } diff --git a/packages/medusa-test-utils/src/module-test-runner.ts b/packages/medusa-test-utils/src/module-test-runner.ts index 24e6db96c8..1c5ca7bfca 100644 --- a/packages/medusa-test-utils/src/module-test-runner.ts +++ b/packages/medusa-test-utils/src/module-test-runner.ts @@ -1,13 +1,12 @@ -import { ContainerRegistrationKeys, ModulesSdkUtils } from "@medusajs/utils" -import { InitModulesOptions, initModules } from "./init-modules" -import { MedusaAppOutput, ModulesDefinition } from "@medusajs/modules-sdk" -import { TestDatabase, getDatabaseURL, getMikroOrmWrapper } from "./database" +import { initModules, InitModulesOptions } from "./init-modules" +import { getDatabaseURL, getMikroOrmWrapper, TestDatabase } from "./database" import { MockEventBusService } from "." +import { ContainerRegistrationKeys, ModulesSdkUtils } from "@medusajs/utils" export interface SuiteOptions { MikroOrmWrapper: TestDatabase - medusaApp: MedusaAppOutput + medusaApp: any service: TService dbConfig: { schema: string @@ -37,6 +36,8 @@ export function moduleIntegrationTestRunner({ debug?: boolean testSuite: (options: SuiteOptions) => () => void }) { + const moduleSdkImports = require("@medusajs/modules-sdk") + process.env.LOG_LEVEL = "error" moduleModels ??= Object.values(require(`${process.cwd()}/src/models`)) @@ -62,7 +63,7 @@ export function moduleIntegrationTestRunner({ const modulesConfig_ = { [moduleName]: { - definition: ModulesDefinition[moduleName], + definition: moduleSdkImports.ModulesDefinition[moduleName], resolve, options: { defaultAdapterOptions: { @@ -89,7 +90,7 @@ export function moduleIntegrationTestRunner({ let shutdown: () => Promise let moduleService - let medusaApp: MedusaAppOutput = {} as MedusaAppOutput + let medusaApp = {} const options = { MikroOrmWrapper, @@ -100,7 +101,7 @@ export function moduleIntegrationTestRunner({ return medusaApp[prop] }, } - ) as MedusaAppOutput, + ), service: new Proxy( {}, { @@ -123,7 +124,7 @@ export function moduleIntegrationTestRunner({ await MikroOrmWrapper.clearDatabase() await shutdown() moduleService = {} - medusaApp = {} as MedusaAppOutput + medusaApp = {} } return describe("", () => { diff --git a/packages/modules-sdk/src/loaders/utils/load-internal.ts b/packages/modules-sdk/src/loaders/utils/load-internal.ts index 2c4db5cfc3..bd6fb6abb2 100644 --- a/packages/modules-sdk/src/loaders/utils/load-internal.ts +++ b/packages/modules-sdk/src/loaders/utils/load-internal.ts @@ -108,6 +108,7 @@ export async function loadInternalModule( container: localContainer, logger, options: resolution.options, + dataLoaderOnly: loaderOnly, }, resolution.moduleDeclaration as InternalModuleDeclaration ) diff --git a/packages/modules-sdk/src/medusa-module.ts b/packages/modules-sdk/src/medusa-module.ts index 0ed04f887e..2c03d1b43d 100644 --- a/packages/modules-sdk/src/medusa-module.ts +++ b/packages/modules-sdk/src/medusa-module.ts @@ -67,7 +67,9 @@ export type ModuleBootstrapOptions = { */ migrationOnly?: boolean /** - * Forces the modules bootstrapper to only run the modules loaders and return prematurely + * Forces the modules bootstrapper to only run the modules loaders and return prematurely. This + * is meant for modules that have data loader. In a test env, in order to clear all data + * and load them back, we need to run those loader again */ loaderOnly?: boolean workerMode?: "shared" | "worker" | "server" diff --git a/packages/types/src/modules-sdk/index.ts b/packages/types/src/modules-sdk/index.ts index 7ffc938e61..af5d1fab07 100644 --- a/packages/types/src/modules-sdk/index.ts +++ b/packages/types/src/modules-sdk/index.ts @@ -118,6 +118,7 @@ export type LoaderOptions> = { container: MedusaContainer options?: TOptions logger?: Logger + dataLoaderOnly?: boolean } export type ModuleLoaderFunction = ( diff --git a/packages/workflow-engine-redis/src/loaders/redis.ts b/packages/workflow-engine-redis/src/loaders/redis.ts index 51c0cc3a66..dd8a974c0f 100644 --- a/packages/workflow-engine-redis/src/loaders/redis.ts +++ b/packages/workflow-engine-redis/src/loaders/redis.ts @@ -7,6 +7,7 @@ export default async ({ container, logger, options, + dataLoaderOnly }: LoaderOptions): Promise => { const { url, @@ -58,6 +59,7 @@ export default async ({ } container.register({ + partialLoading: asValue(true), redisConnection: asValue(connection), redisWorkerConnection: asValue(workerConnection), redisPublisher: asValue(redisPublisher), diff --git a/packages/workflow-engine-redis/src/loaders/utils.ts b/packages/workflow-engine-redis/src/loaders/utils.ts index f662dc1e17..426ba66063 100644 --- a/packages/workflow-engine-redis/src/loaders/utils.ts +++ b/packages/workflow-engine-redis/src/loaders/utils.ts @@ -1,10 +1,11 @@ -import { asClass } from "awilix" +import { asClass, asValue } from "awilix" import { RedisDistributedTransactionStorage } from "../utils" -export default async ({ container }): Promise => { +export default async ({ container, dataLoaderOnly }): Promise => { container.register({ redisDistributedTransactionStorage: asClass( RedisDistributedTransactionStorage ).singleton(), + dataLoaderOnly: asValue(!!dataLoaderOnly), }) } diff --git a/packages/workflow-engine-redis/src/services/workflow-orchestrator.ts b/packages/workflow-engine-redis/src/services/workflow-orchestrator.ts index 40d12b1bb1..e4a7fdcdb1 100644 --- a/packages/workflow-engine-redis/src/services/workflow-orchestrator.ts +++ b/packages/workflow-engine-redis/src/services/workflow-orchestrator.ts @@ -79,10 +79,12 @@ export class WorkflowOrchestratorService { protected redisDistributedTransactionStorage_: RedisDistributedTransactionStorage constructor({ + dataLoaderOnly, redisDistributedTransactionStorage, redisPublisher, redisSubscriber, }: { + dataLoaderOnly: boolean redisDistributedTransactionStorage: RedisDistributedTransactionStorage workflowOrchestratorService: WorkflowOrchestratorService redisPublisher: Redis @@ -92,7 +94,10 @@ export class WorkflowOrchestratorService { this.redisSubscriber = redisSubscriber redisDistributedTransactionStorage.setWorkflowOrchestratorService(this) - DistributedTransaction.setStorage(redisDistributedTransactionStorage) + + if (!dataLoaderOnly) { + DistributedTransaction.setStorage(redisDistributedTransactionStorage) + } this.redisDistributedTransactionStorage_ = redisDistributedTransactionStorage diff --git a/yarn.lock b/yarn.lock index 28deb32ae9..794f2f19d6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -38528,7 +38528,6 @@ __metadata: version: 0.0.0-use.local resolution: "medusa-test-utils@workspace:packages/medusa-test-utils" dependencies: - "@medusajs/modules-sdk": ^1.12.10 "@medusajs/types": ^1.11.15 "@medusajs/utils": ^1.11.8 "@mikro-orm/migrations": 5.9.7 @@ -38541,6 +38540,7 @@ __metadata: typescript: ^5.1.6 peerDependencies: "@medusajs/medusa": ">1.19" + "@medusajs/modules-sdk": ^1.12.10 axios: ^0.28.0 express: ^4.18.3 get-port: ^5.1.0