From 2f00bd100a9fa97a64cb668fcd3c627a5e45591f Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Wed, 19 Oct 2022 18:22:42 +0200 Subject: [PATCH] chore(medusa): Move formatException to the errorHandler to be always applied and not have to apply it manually (#2467) **What** Move the usage of the formatException to the errorHandler level in order to not have to try catch here and there to apply it. Also make our error handling uniformed and avoid forgetting to apply it. FIXES CORE-721 --- .../src/api/middlewares/error-handler.ts | 4 + .../medusa/src/services/customer-group.ts | 49 +++++----- packages/medusa/src/services/customer.ts | 92 +++++++++--------- packages/medusa/src/services/discount.ts | 59 ++++++------ packages/medusa/src/services/price-list.ts | 63 ++++++------- .../medusa/src/services/product-collection.ts | 23 ++--- packages/medusa/src/services/product.ts | 93 +++++++++---------- .../medusa/src/utils/exception-formatter.ts | 2 +- 8 files changed, 184 insertions(+), 201 deletions(-) diff --git a/packages/medusa/src/api/middlewares/error-handler.ts b/packages/medusa/src/api/middlewares/error-handler.ts index 45ff77e282..36ab5b639f 100644 --- a/packages/medusa/src/api/middlewares/error-handler.ts +++ b/packages/medusa/src/api/middlewares/error-handler.ts @@ -1,6 +1,7 @@ import { NextFunction, Request, Response } from "express" import { MedusaError } from "medusa-core-utils" import { Logger } from "../../types/global" +import { formatException } from "../../utils"; const QUERY_RUNNER_RELEASED = "QueryRunnerAlreadyReleasedError" const TRANSACTION_STARTED = "TransactionAlreadyStartedError" @@ -18,6 +19,9 @@ export default () => { next: NextFunction ) => { const logger: Logger = req.scope.resolve("logger") + + err = formatException(err) + logger.error(err) const errorType = err.type || err.name diff --git a/packages/medusa/src/services/customer-group.ts b/packages/medusa/src/services/customer-group.ts index 38c6e621c2..3854007951 100644 --- a/packages/medusa/src/services/customer-group.ts +++ b/packages/medusa/src/services/customer-group.ts @@ -10,7 +10,6 @@ import { FindConfig, Selector } from "../types/common" import { CustomerGroupUpdate } from "../types/customer-groups" import { buildQuery, - formatException, isDefined, isString, PostgresError, @@ -108,26 +107,8 @@ class CustomerGroupService extends TransactionBaseService { ) return await cgRepo.addCustomers(id, ids) }, - async (error: any) => { - if (error.code === PostgresError.FOREIGN_KEY_ERROR) { - await this.retrieve(id) - - const existingCustomers = await this.customerService_.list({ - id: ids, - }) - - const nonExistingCustomers = ids.filter( - (cId) => existingCustomers.findIndex((el) => el.id === cId) === -1 - ) - - throw new MedusaError( - MedusaError.Types.NOT_FOUND, - `The following customer ids do not exist: ${JSON.stringify( - nonExistingCustomers.join(", ") - )}` - ) - } - throw formatException(error) + async (e: any) => { + await this.handleCreationFail(id, ids, e) } ) } @@ -274,6 +255,32 @@ class CustomerGroupService extends TransactionBaseService { return customerGroup } + + private async handleCreationFail( + id: string, + ids: string[], + error: any + ): Promise { + if (error.code === PostgresError.FOREIGN_KEY_ERROR) { + await this.retrieve(id) + + const existingCustomers = await this.customerService_.list({ + id: ids, + }) + + const nonExistingCustomers = ids.filter( + (cId) => existingCustomers.findIndex((el) => el.id === cId) === -1 + ) + + throw new MedusaError( + MedusaError.Types.NOT_FOUND, + `The following customer ids do not exist: ${JSON.stringify( + nonExistingCustomers.join(", ") + )}` + ) + } + throw error + } } export default CustomerGroupService diff --git a/packages/medusa/src/services/customer.ts b/packages/medusa/src/services/customer.ts index 90eb54e7fe..7a8c63ea8d 100644 --- a/packages/medusa/src/services/customer.ts +++ b/packages/medusa/src/services/customer.ts @@ -10,7 +10,6 @@ import { CustomerRepository } from "../repositories/customer" import { AddressCreatePayload, FindConfig, Selector } from "../types/common" import { CreateCustomerInput, UpdateCustomerInput } from "../types/customers" import { buildQuery, isDefined, setMetadata } from "../utils" -import { formatException } from "../utils/exception-formatter" import EventBusService from "./event-bus" type InjectedDependencies = { @@ -19,6 +18,7 @@ type InjectedDependencies = { customerRepository: typeof CustomerRepository addressRepository: typeof AddressRepository } + /** * Provides layer to manipulate customers. */ @@ -301,57 +301,53 @@ class CustomerService extends TransactionBaseService { customerId: string, update: UpdateCustomerInput ): Promise { - return await this.atomicPhase_( - async (manager) => { - const customerRepository = manager.getCustomRepository( - this.customerRepository_ - ) + return await this.atomicPhase_(async (manager) => { + const customerRepository = manager.getCustomRepository( + this.customerRepository_ + ) - const customer = await this.retrieve(customerId) + const customer = await this.retrieve(customerId) - const { - password, - metadata, - billing_address, - billing_address_id, - groups, - ...rest - } = update + const { + password, + metadata, + billing_address, + billing_address_id, + groups, + ...rest + } = update - if (metadata) { - customer.metadata = setMetadata(customer, metadata) - } - - if ("billing_address_id" in update || "billing_address" in update) { - const address = billing_address_id || billing_address - if (isDefined(address)) { - await this.updateBillingAddress_(customer, address) - } - } - - for (const [key, value] of Object.entries(rest)) { - customer[key] = value - } - - if (password) { - customer.password_hash = await this.hashPassword_(password) - } - - if (groups) { - customer.groups = groups as CustomerGroup[] - } - - const updated = await customerRepository.save(customer) - - await this.eventBusService_ - .withTransaction(manager) - .emit(CustomerService.Events.UPDATED, updated) - return updated - }, - async (error) => { - throw formatException(error) + if (metadata) { + customer.metadata = setMetadata(customer, metadata) } - ) + + if ("billing_address_id" in update || "billing_address" in update) { + const address = billing_address_id || billing_address + if (isDefined(address)) { + await this.updateBillingAddress_(customer, address) + } + } + + for (const [key, value] of Object.entries(rest)) { + customer[key] = value + } + + if (password) { + customer.password_hash = await this.hashPassword_(password) + } + + if (groups) { + customer.groups = groups as CustomerGroup[] + } + + const updated = await customerRepository.save(customer) + + await this.eventBusService_ + .withTransaction(manager) + .emit(CustomerService.Events.UPDATED, updated) + + return updated + }) } /** diff --git a/packages/medusa/src/services/discount.ts b/packages/medusa/src/services/discount.ts index 87d7093580..136507bccc 100644 --- a/packages/medusa/src/services/discount.ts +++ b/packages/medusa/src/services/discount.ts @@ -37,7 +37,6 @@ import { } from "../types/discount" import { buildQuery, setMetadata } from "../utils" import { isFuture, isPast } from "../utils/date-helpers" -import { formatException } from "../utils/exception-formatter" import { FlagRouter } from "../utils/flag-router" import CustomerService from "./customer" import DiscountConditionService from "./discount-condition" @@ -200,39 +199,35 @@ class DiscountService extends TransactionBaseService { "Fixed discounts can have one region" ) } - try { - if (discount.regions) { - discount.regions = (await Promise.all( - discount.regions.map(async (regionId) => - this.regionService_.withTransaction(manager).retrieve(regionId) - ) - )) as Region[] - } - - const discountRule = ruleRepo.create(validatedRule) - const createdDiscountRule = await ruleRepo.save(discountRule) - - const created: Discount = discountRepo.create( - discount as DeepPartial - ) - created.rule = createdDiscountRule - - const result = await discountRepo.save(created) - - if (conditions?.length) { - await Promise.all( - conditions.map(async (cond) => { - await this.discountConditionService_ - .withTransaction(manager) - .upsertCondition({ rule_id: result.rule_id, ...cond }) - }) + if (discount.regions) { + discount.regions = (await Promise.all( + discount.regions.map(async (regionId) => + this.regionService_.withTransaction(manager).retrieve(regionId) ) - } - - return result - } catch (error) { - throw formatException(error) + )) as Region[] } + + const discountRule = ruleRepo.create(validatedRule) + const createdDiscountRule = await ruleRepo.save(discountRule) + + const created: Discount = discountRepo.create( + discount as DeepPartial + ) + created.rule = createdDiscountRule + + const result = await discountRepo.save(created) + + if (conditions?.length) { + await Promise.all( + conditions.map(async (cond) => { + await this.discountConditionService_ + .withTransaction(manager) + .upsertCondition({ rule_id: result.rule_id, ...cond }) + }) + ) + } + + return result }) } diff --git a/packages/medusa/src/services/price-list.ts b/packages/medusa/src/services/price-list.ts index 1364b5304e..af697586a1 100644 --- a/packages/medusa/src/services/price-list.ts +++ b/packages/medusa/src/services/price-list.ts @@ -15,7 +15,6 @@ import { PriceListPriceUpdateInput, UpdatePriceListInput, } from "../types/price-list" -import { formatException } from "../utils/exception-formatter" import ProductService from "./product" import RegionService from "./region" import { TransactionBaseService } from "../interfaces" @@ -119,40 +118,36 @@ class PriceListService extends TransactionBaseService { const { prices, customer_groups, includes_tax, ...rest } = priceListObject - try { - const rawPriceList: DeepPartial = { - ...rest, - } - - if ( - this.featureFlagRouter_.isFeatureEnabled( - TaxInclusivePricingFeatureFlag.key - ) - ) { - if (typeof includes_tax !== "undefined") { - rawPriceList.includes_tax = includes_tax - } - } - - const entity = priceListRepo.create(rawPriceList) - - const priceList = await priceListRepo.save(entity) - - if (prices) { - const prices_ = await this.addCurrencyFromRegion(prices) - await moneyAmountRepo.addPriceListPrices(priceList.id, prices_) - } - - if (customer_groups) { - await this.upsertCustomerGroups_(priceList.id, customer_groups) - } - - return await this.retrieve(priceList.id, { - relations: ["prices", "customer_groups"], - }) - } catch (error) { - throw formatException(error) + const rawPriceList: DeepPartial = { + ...rest, } + + if ( + this.featureFlagRouter_.isFeatureEnabled( + TaxInclusivePricingFeatureFlag.key + ) + ) { + if (typeof includes_tax !== "undefined") { + rawPriceList.includes_tax = includes_tax + } + } + + const entity = priceListRepo.create(rawPriceList) + + const priceList = await priceListRepo.save(entity) + + if (prices) { + const prices_ = await this.addCurrencyFromRegion(prices) + await moneyAmountRepo.addPriceListPrices(priceList.id, prices_) + } + + if (customer_groups) { + await this.upsertCustomerGroups_(priceList.id, customer_groups) + } + + return await this.retrieve(priceList.id, { + relations: ["prices", "customer_groups"], + }) }) } diff --git a/packages/medusa/src/services/product-collection.ts b/packages/medusa/src/services/product-collection.ts index 6935179a6e..91001703d5 100644 --- a/packages/medusa/src/services/product-collection.ts +++ b/packages/medusa/src/services/product-collection.ts @@ -10,7 +10,6 @@ import { UpdateProductCollection, } from "../types/product-collection" import { buildQuery, isString, setMetadata } from "../utils" -import { formatException } from "../utils/exception-formatter" import EventBusService from "./event-bus" type InjectedDependencies = { @@ -113,12 +112,8 @@ class ProductCollectionService extends TransactionBaseService { this.productCollectionRepository_ ) - try { - const productCollection = collectionRepo.create(collection) - return await collectionRepo.save(productCollection) - } catch (error) { - throw formatException(error) - } + const productCollection = collectionRepo.create(collection) + return await collectionRepo.save(productCollection) }) } @@ -183,17 +178,13 @@ class ProductCollectionService extends TransactionBaseService { return await this.atomicPhase_(async (manager) => { const productRepo = manager.getCustomRepository(this.productRepository_) - try { - const { id } = await this.retrieve(collectionId, { select: ["id"] }) + const { id } = await this.retrieve(collectionId, { select: ["id"] }) - await productRepo.bulkAddToCollection(productIds, id) + await productRepo.bulkAddToCollection(productIds, id) - return await this.retrieve(id, { - relations: ["products"], - }) - } catch (error) { - throw formatException(error) - } + return await this.retrieve(id, { + relations: ["products"], + }) }) } diff --git a/packages/medusa/src/services/product.ts b/packages/medusa/src/services/product.ts index 7c33aab648..9a22d69354 100644 --- a/packages/medusa/src/services/product.ts +++ b/packages/medusa/src/services/product.ts @@ -32,7 +32,6 @@ import { UpdateProductInput, } from "../types/product" import { buildQuery, isDefined, setMetadata } from "../utils" -import { formatException } from "../utils/exception-formatter" import EventBusService from "./event-bus" type InjectedDependencies = { @@ -362,61 +361,57 @@ class ProductService extends TransactionBaseService { rest.discountable = false } - try { - let product = productRepo.create(rest) + let product = productRepo.create(rest) - if (images?.length) { - product.images = await imageRepo.upsertImages(images) - } + if (images?.length) { + product.images = await imageRepo.upsertImages(images) + } - if (tags?.length) { - product.tags = await productTagRepo.upsertTags(tags) - } + if (tags?.length) { + product.tags = await productTagRepo.upsertTags(tags) + } - if (typeof type !== `undefined`) { - product.type_id = (await productTypeRepo.upsertType(type))?.id || null - } + if (typeof type !== `undefined`) { + product.type_id = (await productTypeRepo.upsertType(type))?.id || null + } - if ( - this.featureFlagRouter_.isFeatureEnabled(SalesChannelFeatureFlag.key) - ) { - if (isDefined(salesChannels)) { - product.sales_channels = [] - if (salesChannels?.length) { - const salesChannelIds = salesChannels?.map((sc) => sc.id) - product.sales_channels = salesChannelIds?.map( - (id) => ({ id } as SalesChannel) - ) - } + if ( + this.featureFlagRouter_.isFeatureEnabled(SalesChannelFeatureFlag.key) + ) { + if (isDefined(salesChannels)) { + product.sales_channels = [] + if (salesChannels?.length) { + const salesChannelIds = salesChannels?.map((sc) => sc.id) + product.sales_channels = salesChannelIds?.map( + (id) => ({ id } as SalesChannel) + ) } } - - product = await productRepo.save(product) - - product.options = await Promise.all( - (options ?? []).map(async (option) => { - const res = optionRepo.create({ - ...option, - product_id: product.id, - }) - await optionRepo.save(res) - return res - }) - ) - - const result = await this.retrieve(product.id, { - relations: ["options"], - }) - - await this.eventBus_ - .withTransaction(manager) - .emit(ProductService.Events.CREATED, { - id: result.id, - }) - return result - } catch (error) { - throw formatException(error) } + + product = await productRepo.save(product) + + product.options = await Promise.all( + (options ?? []).map(async (option) => { + const res = optionRepo.create({ + ...option, + product_id: product.id, + }) + await optionRepo.save(res) + return res + }) + ) + + const result = await this.retrieve(product.id, { + relations: ["options"], + }) + + await this.eventBus_ + .withTransaction(manager) + .emit(ProductService.Events.CREATED, { + id: result.id, + }) + return result }) } diff --git a/packages/medusa/src/utils/exception-formatter.ts b/packages/medusa/src/utils/exception-formatter.ts index b1b52ad299..8b361b9eeb 100644 --- a/packages/medusa/src/utils/exception-formatter.ts +++ b/packages/medusa/src/utils/exception-formatter.ts @@ -4,7 +4,7 @@ export enum PostgresError { DUPLICATE_ERROR = "23505", FOREIGN_KEY_ERROR = "23503", } -export const formatException = (err): Error => { +export const formatException = (err): MedusaError => { switch (err.code) { case PostgresError.DUPLICATE_ERROR: return new MedusaError(