Improve errors handling in Invoices app (#664)

* Remove migration code, add Sentry breadcrumbs

* Add Sentry breadcrumbs to invoice created webhook

* Add changeset

* remove insecure logs

.changeset/famous-trees-work.md

@@ -0,0 +1,5 @@
+---
+"saleor-app-invoices": patch
+---
+
+Improved error handling in Webhook INVOICE_CREATED. Now Sentry will gather additional breadcrumbs for better debugging. No PII is logged

apps/invoices/next.config.js

@@ -1,7 +1,8 @@
 const { withSentryConfig } = require("@sentry/nextjs");
 
-const isSentryPropertiesInEnvironment =
+const isSentryPropertiesInEnvironment = Boolean(
-  process.env.SENTRY_AUTH_TOKEN && process.env.SENTRY_PROJECT && process.env.SENTRY_ORG;
+  process.env.SENTRY_AUTH_TOKEN && process.env.SENTRY_PROJECT && process.env.SENTRY_ORG
+);
 
 /** @type {import('next').NextConfig} */
 const nextConfig = {
@@ -25,4 +26,6 @@ const configWithSentry = withSentryConfig(
   }
 );
 
+
 module.exports = isSentryPropertiesInEnvironment ? configWithSentry : nextConfig;
+

apps/invoices/sentry.client.config.ts

@@ -9,22 +9,10 @@ import pkg from "./package.json";
 
 Sentry.init({
   dsn: process.env.NEXT_PUBLIC_SENTRY_DSN,
-
-  // Adjust this value in production, or use tracesSampler for greater control
   tracesSampleRate: 0.5,
-
-  // Setting this option to true will print useful information to the console while you're setting up Sentry.
   debug: false,
-
   replaysOnErrorSampleRate: 1.0,
-
-  /*
-   * This sets the sample rate to be 10%. You may want this to be 100% while
-   * in development and sample at a lower rate in production
-   */
   replaysSessionSampleRate: 0.1,
-
-  // You can remove this option if you're not planning to use the Sentry Session Replay feature:
   integrations: [
     new Sentry.Replay({
       // Additional Replay configuration goes in here, for example:

apps/invoices/sentry.edge.config.ts

@@ -10,11 +10,7 @@ import pkg from "./package.json";
 
 Sentry.init({
   dsn: process.env.NEXT_PUBLIC_SENTRY_DSN,
-
-  // Adjust this value in production, or use tracesSampler for greater control
   tracesSampleRate: 0.5,
-
-  // Setting this option to true will print useful information to the console while you're setting up Sentry.
   debug: false,
   environment: process.env.SENTRY_ENVIRONMENT,
   release: `${pkg.name}@${pkg.version}`,

apps/invoices/sentry.server.config.ts

@@ -9,11 +9,7 @@ import pkg from "./package.json";
 
 Sentry.init({
   dsn: process.env.NEXT_PUBLIC_SENTRY_DSN,
-
-  // Adjust this value in production, or use tracesSampler for greater control
   tracesSampleRate: 0.5,
-
-  // Setting this option to true will print useful information to the console while you're setting up Sentry.
   debug: false,
   environment: process.env.SENTRY_ENVIRONMENT,
   release: `${pkg.name}@${pkg.version}`,

apps/invoices/src/modules/app-configuration/app-configuration-router.ts

@@ -7,6 +7,7 @@ import { AppConfigV2MetadataManager } from "./schema-v2/app-config-v2-metadata-m
 import { GetAppConfigurationV2Service } from "./schema-v2/get-app-configuration.v2.service";
 import { ConfigV1ToV2MigrationService } from "./schema-v2/config-v1-to-v2-migration.service";
 import { AddressV2Schema } from "./schema-v2/app-config-schema.v2";
+import { AppConfigV2 } from "./schema-v2/app-config";
 
 const UpsertAddressSchema = z.object({
   address: AddressV2Schema,
@@ -19,19 +20,8 @@ export const appConfigurationRouter = router({
 
     logger.debug("appConfigurationRouterV2.fetch called");
 
-    const appConfigV2 = await new GetAppConfigurationV2Service(ctx).getConfiguration();
+    const appConfigV2 =
-
+      (await new GetAppConfigurationV2Service(ctx).getConfiguration()) ?? new AppConfigV2();
-    /**
-     * MIGRATION CODE START - remove when metadata migrated
-     */
-    if (!appConfigV2) {
-      const migrationService = new ConfigV1ToV2MigrationService(ctx.apiClient, ctx.saleorApiUrl);
-
-      return migrationService.migrate().then((config) => config.getChannelsOverrides());
-    }
-    /**
-     * MIGRATION CODE END
-     */
 
     return appConfigV2.getChannelsOverrides();
   }),
@@ -41,23 +31,8 @@ export const appConfigurationRouter = router({
     })
     .input(UpsertAddressSchema)
     .mutation(async ({ ctx, input }) => {
-      const appConfigV2 = await new GetAppConfigurationV2Service(ctx).getConfiguration();
+      const appConfigV2 =
-
+        (await new GetAppConfigurationV2Service(ctx).getConfiguration()) ?? new AppConfigV2();
-      /**
-       * MIGRATION CODE START - remove when metadata migrated
-       */
-      if (!appConfigV2) {
-        const migrationService = new ConfigV1ToV2MigrationService(ctx.apiClient, ctx.saleorApiUrl);
-
-        await migrationService.migrate((config) =>
-          config.upsertOverride(input.channelSlug, input.address)
-        );
-
-        return;
-      }
-      /**
-       * MIGRATION CODE END
-       */
 
       appConfigV2.upsertOverride(input.channelSlug, input.address);
 
@@ -75,21 +50,8 @@ export const appConfigurationRouter = router({
       })
     )
     .mutation(async ({ ctx, input }) => {
-      const appConfigV2 = await new GetAppConfigurationV2Service(ctx).getConfiguration();
+      const appConfigV2 =
-
+        (await new GetAppConfigurationV2Service(ctx).getConfiguration()) ?? new AppConfigV2();
-      /**
-       * MIGRATION CODE START - remove when metadata migrated
-       */
-      if (!appConfigV2) {
-        const migrationService = new ConfigV1ToV2MigrationService(ctx.apiClient, ctx.saleorApiUrl);
-
-        await migrationService.migrate((config) => config.removeOverride(input.channelSlug));
-
-        return;
-      }
-      /**
-       * MIGRATION CODE END
-       */
 
       appConfigV2.removeOverride(input.channelSlug);
 

apps/invoices/src/modules/app-configuration/ui/address-form.tsx

@@ -181,8 +181,6 @@ export const ConnectedAddressForm = (props: Props) => {
     push("/configuration");
   }, [push]);
 
-  console.log(addressData);
-
   if (channelOverrideConfigQuery.isLoading) {
     return <Text color={"textNeutralSubdued"}>Loading</Text>;
   }

apps/invoices/src/pages/api/webhooks/invoice-requested.ts

@@ -26,6 +26,9 @@ import {
 import { ConfigV1ToV2MigrationService } from "../../../modules/app-configuration/schema-v2/config-v1-to-v2-migration.service";
 import { shopInfoQueryToAddressShape } from "../../../modules/shop-info/shop-info-query-to-address-shape";
 
+import * as Sentry from "@sentry/nextjs";
+import { AppConfigV2 } from "../../../modules/app-configuration/schema-v2/app-config";
+
 const OrderPayload = gql`
   fragment Address on Address {
     id
@@ -158,6 +161,10 @@ export const handler: NextWebhookApiHandler<InvoiceRequestedPayloadFragment> = a
   const { authData, payload, baseUrl } = context;
   const logger = createLogger({ domain: authData.saleorApiUrl, url: baseUrl });
 
+  Sentry.configureScope((s) => {
+    s.setTag("saleorApiUrl", authData.saleorApiUrl);
+  });
+
   const order = payload.order;
 
   logger.info({ orderId: order.id }, "Received event INVOICE_REQUESTED");
@@ -172,6 +179,14 @@ export const handler: NextWebhookApiHandler<InvoiceRequestedPayloadFragment> = a
     InvoiceNumberGenerationStrategy.localizedDate("en-US") // todo connect locale -> where from?
   );
 
+  Sentry.addBreadcrumb({
+    message: "Calculated invoice name",
+    data: {
+      invoiceName: invoiceName,
+    },
+    level: "debug",
+  });
+
   logger.debug({ invoiceName }, "Generated invoice name");
 
   try {
@@ -189,22 +204,19 @@ export const handler: NextWebhookApiHandler<InvoiceRequestedPayloadFragment> = a
 
     logger.debug({ tempPdfLocation }, "Resolved PDF location for temporary files");
 
-    let appConfigV2 = await new GetAppConfigurationV2Service({
+    Sentry.addBreadcrumb({
-      saleorApiUrl: authData.saleorApiUrl,
+      message: "Calculated invoice file location",
-      apiClient: client,
+      data: {
-    }).getConfiguration();
+        invoiceFile: tempPdfLocation,
+      },
+      level: "debug",
+    });
 
-    /**
+    let appConfigV2 =
-     * MIGRATION CODE START - remove when metadata migrated
+      (await new GetAppConfigurationV2Service({
-     */
+        saleorApiUrl: authData.saleorApiUrl,
-    if (!appConfigV2) {
+        apiClient: client,
-      const migrationService = new ConfigV1ToV2MigrationService(client, authData.saleorApiUrl);
+      }).getConfiguration()) ?? new AppConfigV2();
-
-      appConfigV2 = await migrationService.migrate();
-    }
-    /**
-     * MIGRATION CODE END
-     */
 
     const address: AddressV2Shape | null =
       appConfigV2.getChannelsOverrides()[order.channel.slug] ??
@@ -213,6 +225,11 @@ export const handler: NextWebhookApiHandler<InvoiceRequestedPayloadFragment> = a
     if (!address) {
       // todo disable webhook
 
+      Sentry.addBreadcrumb({
+        message: "Address not configured",
+        level: "debug",
+      });
+
       return res.status(200).end("App not configured");
     }
 
@@ -226,25 +243,49 @@ export const handler: NextWebhookApiHandler<InvoiceRequestedPayloadFragment> = a
       .catch((err) => {
         logger.error(err, "Error generating invoice");
 
+        Sentry.captureException(err);
+
         return res.status(500).json({
           error: "Error generating invoice",
         });
       });
 
+    Sentry.addBreadcrumb({
+      message: "Generated invoice file",
+      level: "debug",
+    });
+
     const uploader = new SaleorInvoiceUploader(client);
 
     const uploadedFileUrl = await uploader.upload(tempPdfLocation, `${invoiceName}.pdf`);
 
-    logger.info({ uploadedFileUrl }, "Uploaded file to storage, will notify Saleor now");
+    Sentry.addBreadcrumb({
+      message: "Uploaded file to Saleor",
+      level: "debug",
+    });
+
+    logger.info("Uploaded file to storage, will notify Saleor now");
+    logger.debug({ uploadedFileUrl });
 
     await new InvoiceCreateNotifier(client).notifyInvoiceCreated(
       orderId,
       invoiceName,
       uploadedFileUrl
     );
+
+    Sentry.addBreadcrumb({
+      message: "Notified Saleor about invoice creation",
+      level: "debug",
+      data: {
+        orderId,
+        invoiceName,
+      },
+    });
   } catch (e) {
     logger.error(e);
 
+    Sentry.captureException(e);
+
     return res.status(500).json({
       error: (e as any)?.message ?? "Error",
     });