From 689732e2ff12b6dfbedd1a61aaa7884fd835f8ed Mon Sep 17 00:00:00 2001 From: Lukasz Ostrowski Date: Thu, 15 Jun 2023 15:27:57 +0200 Subject: [PATCH] Added eslint rule for dangerous logger calls and updates apps --- .changeset/fast-spiders-share.md | 5 ++++ apps/cms/next.config.js | 10 ++++--- .../src/lib/cms/client/clients-execution.ts | 29 +++++++++++++++---- .../src/lib/cms/client/clients-operations.ts | 2 +- apps/cms/src/lib/cms/providers/contentful.ts | 26 ++++++++++------- apps/cms/src/lib/cms/providers/datocms.ts | 10 +++---- apps/cms/src/lib/cms/providers/strapi.ts | 17 ++++++----- apps/cms/turbo.json | 7 ++++- .../src/pages/api/auth/mailchimp/callback.ts | 2 +- .../category-mapping.router.ts | 2 +- .../src/modules/taxes/active-connection.ts | 6 +++- .../taxjar/taxjar-connection.router.ts | 2 +- .../api/webhooks/checkout-calculate-taxes.ts | 4 +-- .../api/webhooks/order-calculate-taxes.ts | 4 +-- .../src/pages/api/webhooks/order-created.ts | 10 ++++--- .../src/pages/api/webhooks/order-fulfilled.ts | 8 ++--- packages/eslint-config-saleor/package.json | 4 +-- 17 files changed, 95 insertions(+), 53 deletions(-) create mode 100644 .changeset/fast-spiders-share.md diff --git a/.changeset/fast-spiders-share.md b/.changeset/fast-spiders-share.md new file mode 100644 index 0000000..74ac14e --- /dev/null +++ b/.changeset/fast-spiders-share.md @@ -0,0 +1,5 @@ +--- +"eslint-config-saleor": minor +--- + +Added eslint-plugin-saleor configuration, that errors dangerous logger statements. This should reduce risk of printing too much in logs diff --git a/apps/cms/next.config.js b/apps/cms/next.config.js index 964c407..53bbf4e 100644 --- a/apps/cms/next.config.js +++ b/apps/cms/next.config.js @@ -1,7 +1,9 @@ -// This file sets a custom webpack configuration to use your Next.js app -// with Sentry. -// https://nextjs.org/docs/api-reference/next.config.js/introduction -// https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/ +/* + * This file sets a custom webpack configuration to use your Next.js app + * with Sentry. + * https://nextjs.org/docs/api-reference/next.config.js/introduction + * https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/ + */ const { withSentryConfig } = require("@sentry/nextjs"); const isSentryPropertiesInEnvironment = diff --git a/apps/cms/src/lib/cms/client/clients-execution.ts b/apps/cms/src/lib/cms/client/clients-execution.ts index 03b5322..6141b85 100644 --- a/apps/cms/src/lib/cms/client/clients-execution.ts +++ b/apps/cms/src/lib/cms/client/clients-execution.ts @@ -75,7 +75,12 @@ const executeCmsClientOperation = async ({ deletedCmsId: cmsId, }; } catch (error) { - logger.error("Error deleting item", { error }); + logger.error( + { + error: { message: (error as Error).message, stack: (error as Error).stack }, + }, + "Error deleting item" + ); return { error: "Error deleting item.", @@ -104,7 +109,9 @@ const executeCmsClientOperation = async ({ }, }); } catch (error) { - logger.error("Error updating item", { error }); + logger.error("Error updating item", { + error: { message: (error as Error).message, stack: (error as Error).stack }, + }); return { error: "Error updating item.", @@ -137,7 +144,9 @@ const executeCmsClientOperation = async ({ }; } } catch (error) { - logger.error("Error creating item", { error }); + logger.error("Error creating item", { + error: { message: (error as Error).message, stack: (error as Error).stack }, + }); return { error: "Error creating item.", @@ -209,7 +218,12 @@ export const executeCmsClientBatchOperation = async ({ .map((item) => (item as ProductResponseSuccess).data) || [], }; } catch (error) { - logger.error({ error }, "Error creating batch items"); + logger.error( + { + error: { message: (error as Error).message, stack: (error as Error).stack }, + }, + "Error creating batch items" + ); return { error: "Error creating batch items.", @@ -252,7 +266,12 @@ export const executeCmsClientBatchOperation = async ({ deletedCmsIds: CMSIdsToRemove, }; } catch (error) { - logger.error({ error }, "Error removing batch items"); + logger.error( + { + error: { message: (error as Error).message, stack: (error as Error).stack }, + }, + "Error removing batch items" + ); return { error: "Error removing batch items.", diff --git a/apps/cms/src/lib/cms/client/clients-operations.ts b/apps/cms/src/lib/cms/client/clients-operations.ts index f6fd4cd..701946c 100644 --- a/apps/cms/src/lib/cms/client/clients-operations.ts +++ b/apps/cms/src/lib/cms/client/clients-operations.ts @@ -39,7 +39,7 @@ export const createCmsOperations = async ({ getProviderInstancesSettings(settingsManager), ]); - logger.debug({ channelsSettingsParsed, providerInstancesSettingsParsed }, "Fetched settings"); + logger.debug("Fetched settings"); const productVariantCmsProviderInstances = productVariantCmsKeys.map((cmsKey) => getCmsIdFromSaleorItemKey(cmsKey) diff --git a/apps/cms/src/lib/cms/providers/contentful.ts b/apps/cms/src/lib/cms/providers/contentful.ts index 28b68b3..60336f5 100644 --- a/apps/cms/src/lib/cms/providers/contentful.ts +++ b/apps/cms/src/lib/cms/providers/contentful.ts @@ -107,7 +107,8 @@ const contentfulOperations: CreateOperations = (config) => { const response = await contentfulFetch(endpoint, config, { method: "GET" }); const respBody = await response.json(); - logger.debug({ response, body: respBody }, "pingCMS response"); + logger.trace("pingCMS success"); + return { ok: response.ok, }; @@ -130,7 +131,8 @@ const contentfulOperations: CreateOperations = (config) => { }, }); - logger.debug({ response }, "createProduct response"); + logger.trace("createProduct success"); + const json = await response.json(); return { @@ -150,10 +152,11 @@ const contentfulOperations: CreateOperations = (config) => { const getEntryResponse = await contentfulFetch(endpoint, config, { method: "GET" }); - logger.debug({ getEntryResponse }, "updateProduct getEntryResponse"); + logger.trace("updateProduct success"); + const entry = await getEntryResponse.json(); - logger.debug({ entry }, "updateProduct entry"); + logger.trace("updateProduct entry success"); const response = await contentfulFetch(endpoint, config, { method: "PUT", @@ -163,7 +166,8 @@ const contentfulOperations: CreateOperations = (config) => { }, }); - logger.debug({ response }, "updateProduct response"); + logger.trace("updateProduct success"); + const json = await response.json(); return { @@ -226,42 +230,42 @@ const contentfulOperations: CreateOperations = (config) => { ping: async () => { const response = await pingCMS(); - logger.debug({ response }, "ping response"); + logger.trace("ping success"); return response; }, createProduct: async ({ input }) => { const result = await createProductInCMS(input); - logger.debug({ result }, "createProduct result"); + logger.trace("createProduct success"); return transformCreateProductResponse(result); }, updateProduct: async ({ id, input }) => { const result = await updateProductInCMS(id, input); - logger.debug({ result }, "updateProduct result"); + logger.trace("updateProduct result"); return result; }, deleteProduct: async ({ id }) => { const response = await deleteProductInCMS(id); - logger.debug({ response }, "deleteProduct response"); + logger.trace("deleteProduct success"); return response; }, createBatchProducts: async ({ input }) => { const results = await createBatchProductsInCMS(input); - logger.debug({ results }, "createBatchProducts results"); + logger.trace("createBatchProducts success"); return results.map((result) => transformCreateProductResponse(result)); }, deleteBatchProducts: async ({ ids }) => { const results = await deleteBatchProductsInCMS(ids); - logger.debug({ results }, "deleteBatchProducts results"); + logger.trace("deleteBatchProducts success"); }, }; }; diff --git a/apps/cms/src/lib/cms/providers/datocms.ts b/apps/cms/src/lib/cms/providers/datocms.ts index 0859872..cb526e9 100644 --- a/apps/cms/src/lib/cms/providers/datocms.ts +++ b/apps/cms/src/lib/cms/providers/datocms.ts @@ -112,7 +112,7 @@ const datocmsOperations: CreateOperations = (config) => { try { const item = await createProductInCMS(input); - logger.debug({ item }, "createProduct response"); + logger.trace("createProduct success"); return transformResponseItem(item, input); } catch (error) { @@ -122,24 +122,24 @@ const datocmsOperations: CreateOperations = (config) => { updateProduct: async ({ id, input }) => { const item = await updateProductInCMS(id, input); - logger.debug({ item }, "updateProduct response"); + logger.trace("updateProduct success"); }, deleteProduct: async ({ id }) => { const item = await deleteProductInCMS(id); - logger.debug({ item }, "deleteProduct response"); + logger.trace("deleteProduct success"); }, createBatchProducts: async ({ input }) => { const items = await createBatchProductsInCMS(input); - logger.debug({ items }, "createBatchProducts response"); + logger.trace("createBatchProducts success"); return items.map((item) => transformResponseItem(item.id, item.input)); }, deleteBatchProducts: async ({ ids }) => { const items = await deleteBatchProductsInCMS(ids); - logger.debug({ items }, "deleteBatchProducts response"); + logger.trace("deleteBatchProducts success"); }, }; }; diff --git a/apps/cms/src/lib/cms/providers/strapi.ts b/apps/cms/src/lib/cms/providers/strapi.ts index 115e227..85d9a39 100644 --- a/apps/cms/src/lib/cms/providers/strapi.ts +++ b/apps/cms/src/lib/cms/providers/strapi.ts @@ -88,7 +88,8 @@ export const strapiOperations: CreateStrapiOperations = (config) => { method: "GET", }); - logger.debug({ response }, "pingCMS response"); + logger.debug("pingCMS success"); + return { ok: response.ok }; }; @@ -99,7 +100,7 @@ export const strapiOperations: CreateStrapiOperations = (config) => { body: JSON.stringify(body), }); - logger.debug({ response }, "createProduct response"); + logger.debug("createProduct success"); return await response.json(); }; @@ -135,42 +136,42 @@ export const strapiOperations: CreateStrapiOperations = (config) => { ping: async () => { const response = await pingCMS(); - logger.debug({ response }, "ping response"); + logger.debug("ping response"); return response; }, createProduct: async ({ input }) => { const result = await createProductInCMS(input); - logger.debug({ result }, "createProduct result"); + logger.debug("createProduct success"); return transformCreateProductResponse(result, input); }, updateProduct: async ({ id, input }) => { const response = await updateProductInCMS(id, input); - logger.debug({ response }, "updateProduct response"); + logger.debug("updateProduct success"); return response; }, deleteProduct: async ({ id }) => { const response = await deleteProductInCMS(id); - logger.debug({ response }, "deleteProduct response"); + logger.debug("deleteProduct success"); return response; }, createBatchProducts: async ({ input }) => { const results = await createBatchProductsInCMS(input); - logger.debug({ results }, "createBatchProducts results"); + logger.debug("createBatchProducts success"); return results.map((result) => transformCreateProductResponse(result.response, result.input)); }, deleteBatchProducts: async ({ ids }) => { const responses = await deleteBatchProductsInCMS(ids); - logger.debug({ responses }, "deleteBatchProducts responses"); + logger.debug("deleteBatchProducts success"); return responses; }, diff --git a/apps/cms/turbo.json b/apps/cms/turbo.json index 61820a3..f2a62a9 100644 --- a/apps/cms/turbo.json +++ b/apps/cms/turbo.json @@ -12,7 +12,12 @@ "VERCEL_URL", "REST_APL_ENDPOINT", "REST_APL_TOKEN", - "ALLOWED_DOMAIN_PATTERN" + "ALLOWED_DOMAIN_PATTERN", + "SENTRY_AUTH_TOKEN", + "SENTRY_PROJECT", + "SENTRY_ORG", + "SENTRY_DSN", + "NEXT_PUBLIC_SENTRY_DSN" ] } } diff --git a/apps/crm/src/pages/api/auth/mailchimp/callback.ts b/apps/crm/src/pages/api/auth/mailchimp/callback.ts index ae5fcdf..66be35f 100644 --- a/apps/crm/src/pages/api/auth/mailchimp/callback.ts +++ b/apps/crm/src/pages/api/auth/mailchimp/callback.ts @@ -30,7 +30,7 @@ const handler: NextApiHandler = async (req, res) => { const { access_token } = await tokenResponse.json(); - logger.debug({ access_token }, "Received mailchimp access_token"); + logger.debug("Received mailchimp access_token"); const metadataResponse = await fetch("https://login.mailchimp.com/oauth2/metadata", { headers: { diff --git a/apps/products-feed/src/modules/category-mapping/category-mapping.router.ts b/apps/products-feed/src/modules/category-mapping/category-mapping.router.ts index b6001f5..ae4539b 100644 --- a/apps/products-feed/src/modules/category-mapping/category-mapping.router.ts +++ b/apps/products-feed/src/modules/category-mapping/category-mapping.router.ts @@ -41,7 +41,7 @@ export const categoryMappingRouter = router({ logger.debug( { - input, + ...input, }, "Updated category mapping" ); diff --git a/apps/taxes/src/modules/taxes/active-connection.ts b/apps/taxes/src/modules/taxes/active-connection.ts index d8cf197..154723d 100644 --- a/apps/taxes/src/modules/taxes/active-connection.ts +++ b/apps/taxes/src/modules/taxes/active-connection.ts @@ -88,7 +88,11 @@ export function getActiveConnection( if (!providerConnection) { logger.debug( - { providerConnections, channelConfig }, + { + providerConnectionsId: providerConnections.map((c) => c.id), + channelConfigId: channelConfig.config.providerConnectionId, + channelConfigSlug: channelConfig.config.slug, + }, "In the providers array, there is no item with an id that matches the channel config providerConnectionId." ); throw new Error(`Channel config providerConnectionId does not match any providers`); diff --git a/apps/taxes/src/modules/taxjar/taxjar-connection.router.ts b/apps/taxes/src/modules/taxjar/taxjar-connection.router.ts index 3f6510c..ee538a5 100644 --- a/apps/taxes/src/modules/taxjar/taxjar-connection.router.ts +++ b/apps/taxes/src/modules/taxjar/taxjar-connection.router.ts @@ -85,7 +85,7 @@ export const taxjarConnectionRouter = router({ location: "taxjarConnectionRouter.patch", }); - logger.debug({ input }, "Route patch called"); + logger.debug("Route patch called"); const result = await ctx.connectionService.update(input.id, input.value); diff --git a/apps/taxes/src/pages/api/webhooks/checkout-calculate-taxes.ts b/apps/taxes/src/pages/api/webhooks/checkout-calculate-taxes.ts index 62e889c..294b9bc 100644 --- a/apps/taxes/src/pages/api/webhooks/checkout-calculate-taxes.ts +++ b/apps/taxes/src/pages/api/webhooks/checkout-calculate-taxes.ts @@ -56,10 +56,10 @@ export default checkoutCalculateTaxesSyncWebhook.createHandler(async (req, res, const channelSlug = payload.taxBase.channel.slug; const taxProvider = getActiveConnection(channelSlug, appMetadata); - logger.info({ taxProvider }, "Will calculate taxes using the tax provider:"); + logger.info("Will calculate taxes using the tax provider:"); const calculatedTaxes = await taxProvider.calculateTaxes(payload.taxBase); - logger.info({ calculatedTaxes }, "Taxes calculated"); + logger.info("Taxes calculated"); return webhookResponse.success(ctx.buildResponse(calculatedTaxes)); } catch (error) { return webhookResponse.error(error); diff --git a/apps/taxes/src/pages/api/webhooks/order-calculate-taxes.ts b/apps/taxes/src/pages/api/webhooks/order-calculate-taxes.ts index 8c007dc..5e7aed5 100644 --- a/apps/taxes/src/pages/api/webhooks/order-calculate-taxes.ts +++ b/apps/taxes/src/pages/api/webhooks/order-calculate-taxes.ts @@ -56,10 +56,10 @@ export default orderCalculateTaxesSyncWebhook.createHandler(async (req, res, ctx const channelSlug = payload.taxBase.channel.slug; const taxProvider = getActiveConnection(channelSlug, appMetadata); - logger.info({ taxProvider }, "Will calculate taxes using the tax provider:"); + logger.info("Will calculate taxes"); const calculatedTaxes = await taxProvider.calculateTaxes(payload.taxBase); - logger.info({ calculatedTaxes }, "Taxes calculated"); + logger.info("Taxes calculated"); return webhookResponse.success(ctx.buildResponse(calculatedTaxes)); } catch (error) { return webhookResponse.error(error); diff --git a/apps/taxes/src/pages/api/webhooks/order-created.ts b/apps/taxes/src/pages/api/webhooks/order-created.ts index 5f3b772..075a68d 100644 --- a/apps/taxes/src/pages/api/webhooks/order-created.ts +++ b/apps/taxes/src/pages/api/webhooks/order-created.ts @@ -69,14 +69,14 @@ export default orderCreatedAsyncWebhook.createHandler(async (req, res, ctx) => { const { saleorApiUrl, token } = authData; const webhookResponse = new WebhookResponse(res); - logger.info({ payload }, "Handler called with payload"); + logger.info({ orderId: payload?.order?.id }, "Handler called with order ID"); try { const appMetadata = payload.recipient?.privateMetadata ?? []; const channelSlug = payload.order?.channel.slug; const taxProvider = getActiveConnection(channelSlug, appMetadata); - logger.info({ taxProvider }, "Fetched taxProvider"); + logger.info("Fetched taxProvider"); // todo: figure out what fields are needed and add validation if (!payload.order) { @@ -89,7 +89,7 @@ export default orderCreatedAsyncWebhook.createHandler(async (req, res, ctx) => { const createdOrder = await taxProvider.createOrder(payload.order); - logger.info({ createdOrder }, "Order created"); + logger.info({ createdOrderID: createdOrder.id }, "Order created"); const client = createClient(saleorApiUrl, async () => Promise.resolve({ token })); await updateOrderMetadataWithExternalId(client, payload.order.id, createdOrder.id); @@ -97,7 +97,9 @@ export default orderCreatedAsyncWebhook.createHandler(async (req, res, ctx) => { return webhookResponse.success(); } catch (error) { - logger.error({ error }); + logger.error({ + error: { message: (error as Error).message, stack: (error as Error).stack }, + }); return webhookResponse.error(new Error("Error while creating order in tax provider")); } }); diff --git a/apps/taxes/src/pages/api/webhooks/order-fulfilled.ts b/apps/taxes/src/pages/api/webhooks/order-fulfilled.ts index 53acf3d..802d418 100644 --- a/apps/taxes/src/pages/api/webhooks/order-fulfilled.ts +++ b/apps/taxes/src/pages/api/webhooks/order-fulfilled.ts @@ -27,18 +27,18 @@ export const orderFulfilledAsyncWebhook = new SaleorAsyncWebhook { - const logger = createLogger({ event: ctx.event }); + const logger = createLogger({ event: ctx.event, orderId: ctx.payload.order?.id }); const { payload } = ctx; const webhookResponse = new WebhookResponse(res); - logger.info({ payload }, "Handler called with payload"); + logger.info("Handler called"); try { const appMetadata = payload.recipient?.privateMetadata ?? []; const channelSlug = payload.order?.channel.slug; const taxProvider = getActiveConnection(channelSlug, appMetadata); - logger.info({ taxProvider }, "Fetched taxProvider"); + logger.info("Fetched taxProvider"); // todo: figure out what fields are needed and add validation if (!payload.order) { @@ -46,7 +46,7 @@ export default orderFulfilledAsyncWebhook.createHandler(async (req, res, ctx) => } const fulfilledOrder = await taxProvider.fulfillOrder(payload.order); - logger.info({ fulfilledOrder }, "Order fulfilled"); + logger.info("Order fulfilled"); return webhookResponse.success(); } catch (error) { diff --git a/packages/eslint-config-saleor/package.json b/packages/eslint-config-saleor/package.json index 18acf69..33f28c9 100644 --- a/packages/eslint-config-saleor/package.json +++ b/packages/eslint-config-saleor/package.json @@ -2,14 +2,14 @@ "name": "eslint-config-saleor", "version": "0.4.1", "devDependencies": { + "@saleor/eslint-plugin-saleor-app": "0.1.2", "eslint": "8.42.0", "eslint-config-next": "13.3.4", "eslint-config-prettier": "8.8.0", "eslint-config-turbo": "1.10.1", "eslint-plugin-react": "7.32.2", "next": "13.3.0", - "typescript": "5.1.3", - "@saleor/eslint-plugin-saleor-app": "0.1.2" + "typescript": "5.1.3" }, "license": "MIT", "main": "index.js",