feat: Normalize known DB errors to a MedusaError when possible (#6922)

Before we would swallow the error and return a generic error to the user. This will provide more information to the caller if it is one of the known errors.
This commit is contained in:
Stevche Radevski
2024-04-04 19:12:59 +00:00
committed by GitHub
parent e944a627f0
commit 20e8df914e
14 changed files with 282 additions and 169 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"@medusajs/utils": minor
---
Return normalized DB errors from the mikro ORM repository
@@ -2630,8 +2630,7 @@ medusaIntegrationTestRunner({
)
})
// TODO: This needs to be fixed
it.skip("successfully creates product with soft-deleted product handle and deletes it again", async () => {
it("successfully creates product with soft-deleted product handle and deletes it again", async () => {
// First we soft-delete the product
const response = await api
.delete(`/admin/products/${baseProduct.id}`, adminHeaders)
@@ -2708,8 +2707,7 @@ medusaIntegrationTestRunner({
expect(response.data.id).toEqual(baseCollection.id)
})
// TODO: This needs to be fixed, it returns 422 now.
it.skip("successfully creates soft-deleted product collection", async () => {
it("successfully creates soft-deleted product collection", async () => {
const response = await api.delete(
`/admin/collections/${baseCollection.id}`,
adminHeaders
@@ -2749,8 +2747,7 @@ medusaIntegrationTestRunner({
}
})
// TODO: This needs to be fixed
it.skip("successfully creates soft-deleted product variant", async () => {
it("successfully creates soft-deleted product variant", async () => {
const variant = baseProduct.variants[0]
const response = await api.delete(
`/admin/products/${baseProduct.id}/variants/${variant.id}`,
@@ -2771,6 +2768,22 @@ medusaIntegrationTestRunner({
ean: variant.ean,
upc: variant.upc,
barcode: variant.barcode,
...breaking(
() => ({
options: [
{
option_id: baseProduct.options[0].id,
value: "newval",
},
{
option_id: baseProduct.options[1].id,
value: "newval",
},
],
}),
// TODO: Require that options are passed if they belong on the product, and the combos are unique per variant
() => ({})
),
prices: [
{
currency_code: "usd",
@@ -83,7 +83,7 @@ medusaIntegrationTestRunner({
})
})
it("should throw error when shipping option does not exist", async () => {
it.only("should throw error when shipping option does not exist", async () => {
const { response } = await api
.post(
`/admin/fulfillment/shipping-options/does-not-exist/rules/batch/add`,
@@ -100,7 +100,7 @@ medusaIntegrationTestRunner({
expect(response.data).toEqual({
type: "not_found",
message:
"Shipping_option with shipping_option_id does-not-exist does not exist.",
"You tried to set relationship shipping_option_id: does-not-exist, but such entity does not exist",
})
})
@@ -106,7 +106,7 @@ moduleIntegrationTestRunner({
],
}
await expect(service.create(customerData)).rejects.toThrow(
"A default shipping address already exists"
/Customer address with customer_id: .*? already exists./
)
})
@@ -727,7 +727,9 @@ moduleIntegrationTestRunner({
country_code: "US",
is_default_shipping: true,
})
).rejects.toThrow("A default shipping address already exists")
).rejects.toThrow(
/Customer address with customer_id: .*? already exists./
)
})
it("should only be possible to add one default billing address per customer", async () => {
@@ -761,7 +763,9 @@ moduleIntegrationTestRunner({
country_code: "US",
is_default_billing: true,
})
).rejects.toThrow("A default billing address already exists")
).rejects.toThrow(
/Customer address with customer_id: .*? already exists./
)
})
})
@@ -899,7 +903,9 @@ moduleIntegrationTestRunner({
await expect(
service.updateAddresses(address.id, { is_default_shipping: true })
).rejects.toThrow("A default shipping address already exists")
).rejects.toThrow(
/Customer address with customer_id: .*? already exists./
)
})
})
@@ -106,9 +106,7 @@ export default class CustomerModuleService<
| CustomerTypes.CreateCustomerDTO[],
@MedusaContext() sharedContext: Context = {}
): Promise<CustomerTypes.CustomerDTO | CustomerTypes.CustomerDTO[]> {
const customers = await this.create_(dataOrArray, sharedContext).catch(
this.handleDbErrors
)
const customers = await this.create_(dataOrArray, sharedContext)
const serialized = await this.baseRepository_.serialize<
CustomerTypes.CustomerDTO[]
@@ -351,9 +349,7 @@ export default class CustomerModuleService<
): Promise<
CustomerTypes.CustomerAddressDTO | CustomerTypes.CustomerAddressDTO[]
> {
const addresses = await this.addAddresses_(data, sharedContext).catch(
this.handleDbErrors
)
const addresses = await this.addAddresses_(data, sharedContext)
const serialized = await this.baseRepository_.serialize<
CustomerTypes.CustomerAddressDTO[]
@@ -434,7 +430,7 @@ export default class CustomerModuleService<
sharedContext
)
await this.flush(sharedContext).catch(this.handleDbErrors)
await this.flush(sharedContext)
const serialized = await this.baseRepository_.serialize<
CustomerTypes.CustomerAddressDTO[]
@@ -475,25 +471,4 @@ export default class CustomerModuleService<
const em = (context.manager ?? context.transactionManager) as EntityManager
await em.flush()
}
private async handleDbErrors(err: any) {
if (isDuplicateError(err)) {
switch (err.constraint) {
case UNIQUE_CUSTOMER_SHIPPING_ADDRESS:
throw new MedusaError(
MedusaError.Types.DUPLICATE_ERROR,
"A default shipping address already exists"
)
case UNIQUE_CUSTOMER_BILLING_ADDRESS:
throw new MedusaError(
MedusaError.Types.DUPLICATE_ERROR,
"A default billing address already exists"
)
default:
break
}
}
throw err
}
}
@@ -512,7 +512,7 @@ moduleIntegrationTestRunner({
const err = await service.create(data).catch((e) => e)
expect(err).toBeDefined()
expect(err.constraint).toBe("IDX_fulfillment_set_name_unique")
expect(err.message).toContain("exists")
})
it("should fail on creating a new fulfillment set with new service zones and new geo zones that are not valid", async function () {
@@ -837,7 +837,7 @@ moduleIntegrationTestRunner({
const err = await service.update(updateData).catch((e) => e)
expect(err).toBeDefined()
expect(err.constraint).toBe("IDX_fulfillment_set_name_unique")
expect(err.message).toContain("exists")
})
it("should update a collection of fulfillment sets and replace old service zones by new ones", async function () {
@@ -187,7 +187,7 @@ moduleIntegrationTestRunner({
const err = await service.createServiceZones(data).catch((e) => e)
expect(err).toBeDefined()
expect(err.constraint).toBe("IDX_service_zone_name_unique")
expect(err.message).toContain("exists")
})
it("should fail on creating a service zone and new geo zones that are not valid", async function () {
@@ -435,7 +435,7 @@ moduleIntegrationTestRunner({
.catch((e) => e)
expect(err).toBeDefined()
expect(err.constraint).toBe("IDX_service_zone_name_unique")
expect(err.message).toContain("exists")
})
})
})
@@ -73,7 +73,7 @@ moduleIntegrationTestRunner({
.catch((e) => e)
expect(err).toBeDefined()
expect(err.constraint).toBe("IDX_shipping_profile_name_unique")
expect(err.message).toContain("exists")
})
})
})
@@ -711,7 +711,7 @@ moduleIntegrationTestRunner({
],
})
).rejects.toThrowError(
/You are trying to create a Tax Rate Rule for a reference that already exists. Tax Rate id: .*?, reference id: product_id_1./
/Tax rate rule with tax_rate_id: .*?, reference_id: product_id_1 already exists./
)
const rate = await service.create({
@@ -729,7 +729,7 @@ moduleIntegrationTestRunner({
reference_id: "product_id_1",
})
).rejects.toThrowError(
/You are trying to create a Tax Rate Rule for a reference that already exists. Tax Rate id: .*?, reference id: product_id_1./
/Tax rate rule with tax_rate_id: .*?, reference_id: product_id_1 already exists./
)
})
@@ -764,7 +764,7 @@ moduleIntegrationTestRunner({
province_code: "QC",
})
).rejects.toThrowError(
"You are trying to create a Tax Region for (country_code: ca, province_code: qc) but one already exists."
"Tax region with country_code: ca, province_code: qc already exists."
)
})
@@ -797,7 +797,7 @@ moduleIntegrationTestRunner({
is_default: true,
})
).rejects.toThrowError(
/You are trying to create a default tax rate for region: .*? which already has a default tax rate. Unset the current default rate and try again./
/Tax rate with tax_region_id: .*? already exists./
)
})
@@ -21,9 +21,6 @@ import {
} from "@medusajs/utils"
import { TaxProvider, TaxRate, TaxRateRule, TaxRegion } from "@models"
import { entityNameToLinkableKeysMap, joinerConfig } from "../joiner-config"
import { singleDefaultRegionIndexName } from "../models/tax-rate"
import { uniqueRateReferenceIndexName } from "../models/tax-rate-rule"
import { countryCodeProvinceIndexName } from "../models/tax-region"
type InjectedDependencies = {
baseRepository: DAL.RepositoryService
@@ -106,11 +103,7 @@ export default class TaxModuleService<
@MedusaContext() sharedContext: Context = {}
): Promise<TaxTypes.TaxRateDTO[] | TaxTypes.TaxRateDTO> {
const input = Array.isArray(data) ? data : [data]
const rates = await this.create_(input, sharedContext).catch((err) => {
this.handleCreateError(err)
this.handleCreateRulesError(err)
throw err
})
const rates = await this.create_(input, sharedContext)
return Array.isArray(data) ? rates : rates[0]
}
@@ -182,13 +175,7 @@ export default class TaxModuleService<
data: TaxTypes.UpdateTaxRateDTO,
@MedusaContext() sharedContext: Context = {}
): Promise<TaxTypes.TaxRateDTO | TaxTypes.TaxRateDTO[]> {
const rates = await this.update_(selector, data, sharedContext).catch(
(err) => {
this.handleCreateError(err)
this.handleCreateRulesError(err)
throw err
}
)
const rates = await this.update_(selector, data, sharedContext)
const serialized = await this.baseRepository_.serialize<
TaxTypes.TaxRateDTO[]
>(rates, { populate: true })
@@ -322,12 +309,7 @@ export default class TaxModuleService<
@MedusaContext() sharedContext: Context = {}
) {
const input = Array.isArray(data) ? data : [data]
const result = await this.createTaxRegions_(input, sharedContext).catch(
(err) => {
this.handleCreateRegionsError(err)
throw err
}
)
const result = await this.createTaxRegions_(input, sharedContext)
return Array.isArray(data) ? result : result[0]
}
@@ -382,12 +364,7 @@ export default class TaxModuleService<
@MedusaContext() sharedContext: Context = {}
) {
const input = Array.isArray(data) ? data : [data]
const result = await this.createTaxRateRules_(input, sharedContext).catch(
(err) => {
this.handleCreateRulesError(err)
throw err
}
)
const result = await this.createTaxRateRules_(input, sharedContext)
return Array.isArray(data) ? result : result[0]
}
@@ -748,49 +725,6 @@ export default class TaxModuleService<
return code.toLowerCase()
}
private handleCreateRegionsError(err: any) {
if (err.constraint === countryCodeProvinceIndexName) {
const [countryCode, provinceCode] = err.detail
.split("=")[1]
.match(/\(([^)]+)\)/)[1]
.split(",")
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
`You are trying to create a Tax Region for (country_code: ${countryCode.trim()}, province_code: ${provinceCode.trim()}) but one already exists.`
)
}
}
private handleCreateError(err: any) {
if (err.constraint === singleDefaultRegionIndexName) {
// err.detail = Key (tax_region_id)=(txreg_01HQX5E8GEH36ZHJWFYDAFY67P) already exists.
const regionId = err.detail.split("=")[1].match(/\(([^)]+)\)/)[1]
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
`You are trying to create a default tax rate for region: ${regionId} which already has a default tax rate. Unset the current default rate and try again.`
)
}
}
private handleCreateRulesError(err: any) {
if (err.constraint === uniqueRateReferenceIndexName) {
// err.detail == "Key (tax_rate_id, reference_id)=(txr_01HQWRXTC0JK0F02D977WRR45T, product_id_1) already exists."
// We want to extract the ids from the detail string
// i.e. txr_01HQWRXTC0JK0F02D977WRR45T and product_id_1
const [taxRateId, referenceId] = err.detail
.split("=")[1]
.match(/\(([^)]+)\)/)[1]
.split(",")
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
`You are trying to create a Tax Rate Rule for a reference that already exists. Tax Rate id: ${taxRateId.trim()}, reference id: ${referenceId.trim()}.`
)
}
}
// @InjectTransactionManager("baseRepository_")
// async createProvidersOnLoad(@MedusaContext() sharedContext: Context = {}) {
// const providersToLoad = this.container_["tax_providers"] as ITaxProvider[]
@@ -0,0 +1,83 @@
import {
ForeignKeyConstraintViolationException,
InvalidFieldNameException,
NotFoundError,
NotNullConstraintViolationException,
UniqueConstraintViolationException,
} from "@mikro-orm/core"
import { MedusaError, upperCaseFirst } from "../../common"
export const dbErrorMapper = (err: Error) => {
if (err instanceof NotFoundError) {
console.log(err)
throw new MedusaError(MedusaError.Types.NOT_FOUND, err.message)
}
if (err instanceof UniqueConstraintViolationException) {
const info = getConstraintInfo(err)
if (!info) {
throw err
}
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
`${upperCaseFirst(info.table)} with ${info.keys
.map((key, i) => `${key}: ${info.values[i]}`)
.join(", ")} already exists.`
)
}
if (err instanceof NotNullConstraintViolationException) {
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
`Cannot set field '${(err as any).column}' of ${upperCaseFirst(
(err as any).table
)} to null`
)
}
if (err instanceof InvalidFieldNameException) {
const userFriendlyMessage = err.message.match(/(column.*)/)?.[0]
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
userFriendlyMessage ?? err.message
)
}
if (err instanceof ForeignKeyConstraintViolationException) {
const info = getConstraintInfo(err)
throw new MedusaError(
MedusaError.Types.NOT_FOUND,
`You tried to set relationship ${info?.keys.map(
(key, i) => `${key}: ${info.values[i]}`
)}, but such entity does not exist`
)
}
throw err
}
const getConstraintInfo = (err: any) => {
const detail = err.detail as string
if (!detail) {
return null
}
const [keys, values] = detail.match(/\([^\(.]*\)/g) || []
if (!keys || !values) {
return null
}
return {
table: err.table.split("_").join(" "),
keys: keys
.substring(1, keys.length - 1)
.split(",")
.map((k) => k.trim()),
values: values
.substring(1, values.length - 1)
.split(",")
.map((v) => v.trim()),
}
}
@@ -10,6 +10,7 @@ import {
OnInit,
PrimaryKey,
Property,
Unique,
wrap,
} from "@mikro-orm/core"
import { mikroOrmBaseRepositoryFactory } from "../../mikro-orm-repository"
@@ -101,6 +102,7 @@ class Entity3 {
@PrimaryKey()
id: string
@Unique()
@Property()
title: string
@@ -124,44 +126,44 @@ const Entity2Repository = mikroOrmBaseRepositoryFactory<Entity2>(Entity2)
const Entity3Repository = mikroOrmBaseRepositoryFactory<Entity3>(Entity3)
describe("mikroOrmRepository", () => {
let orm!: MikroORM
let manager!: EntityManager
const manager1 = () => {
return new Entity1Repository({ manager: manager.fork() })
}
const manager2 = () => {
return new Entity2Repository({ manager: manager.fork() })
}
const manager3 = () => {
return new Entity3Repository({ manager: manager.fork() })
}
beforeEach(async () => {
await dropDatabase(
{ databaseName: DB_NAME, errorIfNonExist: false },
pgGodCredentials
)
orm = await MikroORM.init({
entities: [Entity1, Entity2],
clientUrl: getDatabaseURL(),
type: "postgresql",
})
const generator = orm.getSchemaGenerator()
await generator.ensureDatabase()
await generator.createSchema()
manager = orm.em.fork()
})
afterEach(async () => {
const generator = orm.getSchemaGenerator()
await generator.dropSchema()
await orm.close(true)
})
describe("upsert with replace", () => {
let orm!: MikroORM
let manager!: EntityManager
const manager1 = () => {
return new Entity1Repository({ manager: manager.fork() })
}
const manager2 = () => {
return new Entity2Repository({ manager: manager.fork() })
}
const manager3 = () => {
return new Entity3Repository({ manager: manager.fork() })
}
beforeEach(async () => {
await dropDatabase(
{ databaseName: DB_NAME, errorIfNonExist: false },
pgGodCredentials
)
orm = await MikroORM.init({
entities: [Entity1, Entity2],
clientUrl: getDatabaseURL(),
type: "postgresql",
})
const generator = orm.getSchemaGenerator()
await generator.ensureDatabase()
await generator.createSchema()
manager = orm.em.fork()
})
afterEach(async () => {
const generator = orm.getSchemaGenerator()
await generator.dropSchema()
await orm.close(true)
})
it("should successfully create a flat entity", async () => {
const entity1 = { id: "1", title: "en1" }
@@ -585,7 +587,7 @@ describe("mikroOrmRepository", () => {
)
})
it("should successfully update, create, and delete subentities an entity with a many-to-many relation", async () => {
it("should successfully create subentities and delete pivot relationships on a many-to-many relation", async () => {
const entity1 = {
id: "1",
title: "en1",
@@ -598,8 +600,11 @@ describe("mikroOrmRepository", () => {
let resp = await manager1().upsertWithReplace([entity1], {
relations: ["entity3"],
})
entity1.title = "newen1"
entity1.entity3 = [{ id: "4", title: "newen3-1" }, { title: "en3-4" }]
// We don't do many-to-many updates, so id: 4 entity should remain unchanged
resp = await manager1().upsertWithReplace([entity1], {
relations: ["entity3"],
})
@@ -619,6 +624,7 @@ describe("mikroOrmRepository", () => {
expect(listedEntities[0].entity3.getItems()).toEqual(
expect.arrayContaining([
expect.objectContaining({
// title: "en3-1",
title: "newen3-1",
}),
expect.objectContaining({
@@ -673,8 +679,11 @@ describe("mikroOrmRepository", () => {
const mainEntity = await manager1().upsertWithReplace([entity1], {
relations: ["entity3"],
})
entity1.title = "newen1"
entity1.entity3 = [{ id: "4", title: "newen3-1" }, { title: "en3-4" }]
// We don't do many-to-many updates, so id: 4 entity should remain unchanged
await manager1().upsertWithReplace([entity1], {
relations: ["entity3"],
})
@@ -703,6 +712,7 @@ describe("mikroOrmRepository", () => {
expect(listedEntities[0].entity3.getItems()).toEqual(
expect.arrayContaining([
expect.objectContaining({
// title: "en3-1",
title: "newen3-1",
}),
expect.objectContaining({
@@ -774,5 +784,73 @@ describe("mikroOrmRepository", () => {
])
)
})
// it("should correctly handle many-to-many upserts with a uniqueness constriant on a non-primary key", async () => {
// const entity1 = {
// id: "1",
// title: "en1",
// entity3: [{ title: "en3-1" }, { title: "en3-2" }] as any,
// }
// await manager1().upsertWithReplace([entity1], {
// relations: ["entity3"],
// })
// await manager1().upsertWithReplace([{ ...entity1, id: "2" }], {
// relations: ["entity3"],
// })
// const listedEntities = await manager1().find({
// where: {},
// options: { populate: ["entity3"] },
// })
// expect(listedEntities).toHaveLength(2)
// expect(listedEntities[0].entity3.getItems()).toEqual(
// listedEntities[1].entity3.getItems()
// )
// })
})
describe("error mapping", () => {
it("should map UniqueConstraintViolationException to MedusaError on upsertWithReplace", async () => {
const entity3 = { title: "en3" }
await manager3().upsertWithReplace([entity3])
const err = await manager3()
.upsertWithReplace([entity3])
.catch((e) => e.message)
expect(err).toEqual("Entity3 with title: en3 already exists.")
})
it("should map NotNullConstraintViolationException MedusaError on upsertWithReplace", async () => {
const entity3 = { title: null }
const err = await manager3()
.upsertWithReplace([entity3])
.catch((e) => e.message)
expect(err).toEqual("Cannot set field 'title' of Entity3 to null")
})
it("should map InvalidFieldNameException MedusaError on upsertWithReplace", async () => {
const entity3 = { othertitle: "en3" }
const err = await manager3()
.upsertWithReplace([entity3])
.catch((e) => e.message)
expect(err).toEqual(
'column "othertitle" of relation "entity3" does not exist'
)
})
it("should map ForeignKeyConstraintViolationException MedusaError on upsertWithReplace", async () => {
const entity2 = { title: "en2", entity1: { id: "1" } }
const err = await manager2()
.upsertWithReplace([entity2])
.catch((e) => e.message)
expect(err).toEqual(
"You tried to set relationship entity1_id: 1, but such entity does not exist"
)
})
})
})
@@ -27,7 +27,6 @@ import { SqlEntityManager } from "@mikro-orm/postgresql"
import {
MedusaError,
arrayDifference,
deepCopy,
isString,
promiseAll,
} from "../../common"
@@ -38,6 +37,7 @@ import {
} from "../utils"
import { mikroOrmUpdateDeletedAtRecursively } from "./utils"
import { mikroOrmSerializer } from "./mikro-orm-serializer"
import { dbErrorMapper } from "./db-error-mapper"
export class MikroOrmBase<T = any> {
readonly manager_: any
@@ -67,8 +67,13 @@ export class MikroOrmBase<T = any> {
transaction?: TManager
} = {}
): Promise<any> {
// @ts-ignore
return await transactionWrapper.bind(this)(task, options)
const freshManager = this.getFreshManager
? this.getFreshManager()
: this.manager_
return await transactionWrapper(freshManager, task, options).catch(
dbErrorMapper
)
}
async serialize<TOutput extends object | object[]>(
@@ -262,6 +267,23 @@ export function mikroOrmBaseRepositoryFactory<T extends object = object>(
constructor(...args: any[]) {
// @ts-ignore
super(...arguments)
return new Proxy(this, {
get: (target, prop) => {
if (typeof target[prop] === "function") {
return (...args) => {
const res = target[prop].bind(target)(...args)
if (res instanceof Promise) {
return res.catch(dbErrorMapper)
}
return res
}
}
return target[prop]
},
})
}
static buildUniqueCompositeKeyValue(keys: string[], data: object) {
@@ -474,7 +496,8 @@ export function mikroOrmBaseRepositoryFactory<T extends object = object>(
)
if (nonexistentRelations.length) {
throw new Error(
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
`Nonexistent relations were passed during upsert: ${nonexistentRelations}`
)
}
+4 -8
View File
@@ -1,8 +1,8 @@
import { isObject } from "../common"
export async function transactionWrapper<TManager = unknown>(
this: any,
task: (transactionManager: unknown) => Promise<any>,
manager: any,
task: (transactionManager: any) => Promise<any>,
{
transaction,
isolationLevel,
@@ -28,12 +28,8 @@ export async function transactionWrapper<TManager = unknown>(
Object.assign(options, { isolationLevel })
}
const freshManager = this.getFreshManager
? this.getFreshManager()
: this.manager_
const transactionMethod =
freshManager.transaction ?? freshManager.transactional
return await transactionMethod.bind(freshManager)(task, options)
const transactionMethod = manager.transaction ?? manager.transactional
return await transactionMethod.bind(manager)(task, options)
}
/**