Skip to content

Commit cc1309d

Browse files
adrien2padrien2p
authored
chore(product): Improve product normalization and fix http router with tracing (#11724)
**What** - Improve product normalization and prevent over fetching data - Fix HTTP router wrap handler with tracing enabled
1 parent e81deb4 commit cc1309d

File tree

15 files changed

+464
-118
lines changed

15 files changed

+464
-118
lines changed

‎.changeset/heavy-items-own.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@medusajs/product": patch
3+
"@medusajs/framework": patch
4+
"@medusajs/medusa": patch
5+
---
6+
7+
chore(product): Improve product normalization

‎packages/core/framework/src/http/__fixtures__/routers/admin/fail/route.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { Request, Response } from "express"
2+
3+
export function GET(req: Request, res: Response) {
4+
throw new Error("Failed")
5+
}

‎packages/core/framework/src/http/__tests__/index.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,22 @@ describe("RoutesLoader", function () {
3636
request = request_
3737
})
3838

39+
it("should be handled by the error handler when a route handler fails", async function () {
40+
const res = await request("GET", "/admin/fail", {
41+
adminSession: {
42+
jwt: {
43+
userId: "admin_user",
44+
},
45+
},
46+
})
47+
48+
expect(res.status).toBe(500)
49+
console.log(res)
50+
expect(res.text).toBe(
51+
'{"code":"unknown_error","type":"unknown_error","message":"An unknown error occurred."}'
52+
)
53+
})
54+
3955
it("should return a status 200 on GET admin/order/:id", async function () {
4056
const res = await request("GET", "/admin/orders/1000", {
4157
adminSession: {

‎packages/core/framework/src/http/__tests__/routes-loader.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@ describe("Routes loader", () => {
99

1010
expect(loader.getRoutes()).toMatchInlineSnapshot(`
1111
[
12+
{
13+
"absolutePath": "${BASE_DIR}/admin/fail/route.ts",
14+
"handler": [Function],
15+
"isRoute": true,
16+
"matcher": "/admin/fail",
17+
"method": "GET",
18+
"optedOutOfAuth": false,
19+
"relativePath": "/admin/fail/route.ts",
20+
"shouldAppendAdminCors": true,
21+
"shouldAppendAuthCors": false,
22+
"shouldAppendStoreCors": false,
23+
},
1224
{
1325
"absolutePath": "${BASE_DIR}/admin/orders/[id]/route.ts",
1426
"handler": [Function],
@@ -205,6 +217,18 @@ describe("Routes loader", () => {
205217

206218
expect(loader.getRoutes()).toMatchInlineSnapshot(`
207219
[
220+
{
221+
"absolutePath": "${BASE_DIR}/admin/fail/route.ts",
222+
"handler": [Function],
223+
"isRoute": true,
224+
"matcher": "/admin/fail",
225+
"method": "GET",
226+
"optedOutOfAuth": false,
227+
"relativePath": "/admin/fail/route.ts",
228+
"shouldAppendAdminCors": true,
229+
"shouldAppendAuthCors": false,
230+
"shouldAppendStoreCors": false,
231+
},
208232
{
209233
"absolutePath": "${BASE_DIR}/admin/orders/[id]/route.ts",
210234
"handler": [Function],

‎packages/core/framework/src/http/router.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,25 +104,25 @@ export class ApiLoader {
104104
if ("isRoute" in route) {
105105
logger.debug(`registering route ${route.method} ${route.matcher}`)
106106
const handler = ApiLoader.traceRoute
107-
? ApiLoader.traceRoute(wrapHandler(route.handler), {
107+
? ApiLoader.traceRoute(route.handler, {
108108
route: route.matcher,
109109
method: route.method,
110110
})
111-
: wrapHandler(route.handler)
111+
: route.handler
112112

113-
this.#app[route.method.toLowerCase()](route.matcher, handler)
113+
this.#app[route.method.toLowerCase()](route.matcher, wrapHandler(handler))
114114
return
115115
}
116116

117117
if (!route.methods) {
118118
logger.debug(`registering global middleware for ${route.matcher}`)
119119
const handler = ApiLoader.traceMiddleware
120-
? (ApiLoader.traceMiddleware(wrapHandler(route.handler), {
120+
? (ApiLoader.traceMiddleware(route.handler, {
121121
route: route.matcher,
122122
}) as RequestHandler)
123-
: (wrapHandler(route.handler) as RequestHandler)
123+
: (route.handler as RequestHandler)
124124

125-
this.#app.use(route.matcher, handler)
125+
this.#app.use(route.matcher, wrapHandler(handler))
126126
return
127127
}
128128

‎packages/medusa/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
"@types/multer": "^1.4.7",
6161
"jest": "^29.7.0",
6262
"rimraf": "^5.0.1",
63+
"supertest": "^4.0.2",
6364
"typescript": "^5.6.2",
6465
"yalc": "1.0.0-pre.53"
6566
},

‎packages/medusa/src/instrumentation/__fixtures__/mocks/index.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { ConfigModule } from "@medusajs/types"
2+
3+
export const customersGlobalMiddlewareMock = jest.fn()
4+
export const customersCreateMiddlewareMock = jest.fn()
5+
export const storeGlobalMiddlewareMock = jest.fn()
6+
7+
export const config = {
8+
projectConfig: {
9+
databaseLogging: false,
10+
http: {
11+
authCors: "http://localhost:9000",
12+
storeCors: "http://localhost:8000",
13+
adminCors: "http://localhost:7001",
14+
jwtSecret: "supersecret",
15+
cookieSecret: "superSecret",
16+
},
17+
},
18+
featureFlags: {},
19+
plugins: [],
20+
} satisfies Partial<ConfigModule>

‎packages/medusa/src/instrumentation/__fixtures__/routers/admin/fail/route.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { MedusaError } from "@medusajs/framework/utils"
2+
import { Request, Response } from "express"
3+
4+
export function GET(req: Request, res: Response) {
5+
throw new MedusaError(MedusaError.Types.INVALID_DATA, "Failed")
6+
}

‎packages/medusa/src/instrumentation/__fixtures__/routers/middlewares.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { defineMiddlewares } from "@medusajs/framework"
2+
3+
export const errorHandlerMock = jest
4+
.fn()
5+
.mockImplementation((err, req, res, next) => {
6+
console.log("errorHandlerMock", err)
7+
return res.status(400).json({
8+
type: err.code.toLowerCase(),
9+
message: err.message,
10+
})
11+
})
12+
13+
export default defineMiddlewares({
14+
errorHandler: (err, req, res, next) => errorHandlerMock(err, req, res, next),
15+
})

‎packages/medusa/src/instrumentation/__fixtures__/server/index.ts

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
import {
2+
moduleLoader,
3+
ModulesDefinition,
4+
registerMedusaModule,
5+
} from "@medusajs/modules-sdk"
6+
import { ContainerRegistrationKeys, generateJwtToken } from "@medusajs/utils"
7+
import { asValue } from "awilix"
8+
import express from "express"
9+
import querystring from "querystring"
10+
import supertest from "supertest"
11+
12+
import { config } from "../mocks"
13+
import { ConfigModule, MedusaContainer } from "@medusajs/types"
14+
import { configManager } from "@medusajs/framework/config"
15+
import {
16+
ApiLoader,
17+
container,
18+
featureFlagsLoader,
19+
logger,
20+
MedusaRequest,
21+
} from "@medusajs/framework"
22+
23+
function asArray(resolvers) {
24+
return {
25+
resolve: (container) =>
26+
resolvers.map((resolver) => container.build(resolver)),
27+
}
28+
}
29+
30+
/**
31+
* Sets up a test server that injects API Routes using the RoutesLoader
32+
*
33+
* @param {String} rootDir - The root directory of the project
34+
*/
35+
export const createServer = async (rootDir) => {
36+
const app = express()
37+
38+
const moduleResolutions = {}
39+
Object.entries(ModulesDefinition).forEach(([moduleKey, module]) => {
40+
moduleResolutions[moduleKey] = registerMedusaModule(
41+
moduleKey,
42+
module.defaultModuleDeclaration,
43+
undefined,
44+
module
45+
)[moduleKey]
46+
})
47+
48+
configManager.loadConfig({
49+
projectConfig: config as unknown as ConfigModule,
50+
baseDir: rootDir,
51+
})
52+
53+
container.registerAdd = function (this: MedusaContainer, name, registration) {
54+
const storeKey = name + "_STORE"
55+
56+
if (this.registrations[storeKey] === undefined) {
57+
this.register(storeKey, asValue([]))
58+
}
59+
const store = this.resolve(storeKey) as Array<any>
60+
61+
if (this.registrations[name] === undefined) {
62+
this.register(name, asArray(store))
63+
}
64+
store.unshift(registration)
65+
66+
return this
67+
}.bind(container)
68+
69+
container.register(ContainerRegistrationKeys.PG_CONNECTION, asValue({}))
70+
container.register("configModule", asValue(config))
71+
container.register({
72+
logger: asValue({
73+
error: () => {},
74+
}),
75+
manager: asValue({}),
76+
})
77+
78+
app.set("trust proxy", 1)
79+
app.use((req, _res, next) => {
80+
req["session"] = {}
81+
const data = req.get("Cookie")
82+
if (data) {
83+
req["session"] = {
84+
...req["session"],
85+
...JSON.parse(data),
86+
}
87+
}
88+
next()
89+
})
90+
91+
await featureFlagsLoader()
92+
await moduleLoader({ container, moduleResolutions, logger })
93+
94+
app.use((req, res, next) => {
95+
;(req as MedusaRequest).scope = container.createScope() as MedusaContainer
96+
next()
97+
})
98+
99+
await new ApiLoader({
100+
app,
101+
sourceDir: rootDir,
102+
}).load()
103+
104+
const superRequest = supertest(app)
105+
106+
return {
107+
request: async (method, url, opts: any = {}) => {
108+
const { payload, query, headers = {} } = opts
109+
110+
const queryParams = query && querystring.stringify(query)
111+
const req = superRequest[method.toLowerCase()](
112+
`${url}${queryParams ? "?" + queryParams : ""}`
113+
)
114+
headers.Cookie = headers.Cookie || ""
115+
if (opts.adminSession) {
116+
const token = generateJwtToken(
117+
{
118+
actor_id: opts.adminSession.userId || opts.adminSession.jwt?.userId,
119+
actor_type: "user",
120+
app_metadata: {
121+
user_id:
122+
opts.adminSession.userId || opts.adminSession.jwt?.userId,
123+
},
124+
},
125+
{
126+
secret: config.projectConfig.http.jwtSecret!,
127+
expiresIn: "1d",
128+
}
129+
)
130+
131+
headers.Authorization = `Bearer ${token}`
132+
}
133+
134+
if (opts.clientSession) {
135+
const token = generateJwtToken(
136+
{
137+
actor_id:
138+
opts.clientSession.customer_id ||
139+
opts.clientSession.jwt?.customer_id,
140+
actor_type: "customer",
141+
app_metadata: {
142+
customer_id:
143+
opts.clientSession.customer_id ||
144+
opts.clientSession.jwt?.customer_id,
145+
},
146+
},
147+
{ secret: config.projectConfig.http.jwtSecret!, expiresIn: "1d" }
148+
)
149+
150+
headers.Authorization = `Bearer ${token}`
151+
}
152+
153+
for (const name in headers) {
154+
if ({}.hasOwnProperty.call(headers, name)) {
155+
req.set(name, headers[name])
156+
}
157+
}
158+
159+
if (payload && !req.get("content-type")) {
160+
req.set("Content-Type", "application/json")
161+
}
162+
163+
if (!req.get("accept")) {
164+
req.set("Accept", "application/json")
165+
}
166+
167+
req.set("Host", "localhost")
168+
169+
let res
170+
try {
171+
res = await req.send(JSON.stringify(payload))
172+
} catch (e) {
173+
if (e.response) {
174+
res = e.response
175+
} else {
176+
throw e
177+
}
178+
}
179+
180+
return res
181+
},
182+
}
183+
}

‎packages/medusa/src/instrumentation/__tests__/index.spec.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { resolve } from "path"
2+
import { errorHandlerMock } from "../__fixtures__/routers/middlewares"
3+
import { createServer } from "../__fixtures__/server"
4+
import { instrumentHttpLayer } from "../index"
5+
import { MedusaError } from "@medusajs/framework/utils"
6+
7+
jest.setTimeout(30000)
8+
9+
jest.mock("../../commands/start", () => {
10+
return {}
11+
})
12+
13+
describe("HTTP Instrumentation", () => {
14+
let request
15+
16+
afterEach(function () {
17+
jest.clearAllMocks()
18+
})
19+
20+
beforeAll(async function () {
21+
instrumentHttpLayer()
22+
23+
const rootDir = resolve(__dirname, "../__fixtures__/routers")
24+
25+
const { request: request_ } = await createServer(rootDir)
26+
27+
request = request_
28+
})
29+
30+
describe("traceRoute", () => {
31+
it("should be handled by the error handler when a route fails", async () => {
32+
const res = await request("GET", "/admin/fail", {
33+
adminSession: {
34+
jwt: {
35+
userId: "admin_user",
36+
},
37+
},
38+
})
39+
40+
expect(res.status).toBe(400)
41+
expect(errorHandlerMock).toHaveBeenCalled()
42+
expect(errorHandlerMock).toHaveBeenCalledWith(
43+
new MedusaError(MedusaError.Types.INVALID_DATA, "Failed"),
44+
expect.anything(),
45+
expect.anything(),
46+
expect.anything()
47+
)
48+
})
49+
})
50+
})

0 commit comments

Comments
 (0)