djkato/saleor-apps-redis_apl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Improve errors handling in Invoices app (#664)

Browse source 
* Remove migration code, add Sentry breadcrumbs

* Add Sentry breadcrumbs to invoice created webhook

* Add changeset

* remove insecure logs
This commit is contained in:
Lukasz Ostrowski 2023-06-27 12:13:04 +02:00 committed by GitHub
parent e4497b9ba4
commit 8b245c6bcf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 74 additions and 85 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/famous-trees-work.md Normal file
View file

@ -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

7
apps/invoices/next.config.js
View file

@ -1,7 +1,8 @@
const { withSentryConfig } = require("@sentry/nextjs");
const isSentryPropertiesInEnvironment =
process.env.SENTRY_AUTH_TOKEN && process.env.SENTRY_PROJECT && process.env.SENTRY_ORG;
const isSentryPropertiesInEnvironment = Boolean(
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;

12
apps/invoices/sentry.client.config.ts
View file

@ -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:

4
apps/invoices/sentry.edge.config.ts
View file

@ -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}`,

4
apps/invoices/sentry.server.config.ts
View file

@ -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}`,

52
apps/invoices/src/modules/app-configuration/app-configuration-router.ts
View file

@ -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();
/**
* 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
*/
const appConfigV2 =
(await new GetAppConfigurationV2Service(ctx).getConfiguration()) ?? new AppConfigV2();
return appConfigV2.getChannelsOverrides();
}),
@ -41,23 +31,8 @@ export const appConfigurationRouter = router({
})
.input(UpsertAddressSchema)
.mutation(async ({ ctx, input }) => {
const appConfigV2 = await new GetAppConfigurationV2Service(ctx).getConfiguration();
/**
* 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
*/
const appConfigV2 =
(await new GetAppConfigurationV2Service(ctx).getConfiguration()) ?? new AppConfigV2();
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();
/**
* 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
*/
const appConfigV2 =
(await new GetAppConfigurationV2Service(ctx).getConfiguration()) ?? new AppConfigV2();
appConfigV2.removeOverride(input.channelSlug);

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

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

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

@ -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({
saleorApiUrl: authData.saleorApiUrl,
apiClient: client,
}).getConfiguration();
Sentry.addBreadcrumb({
message: "Calculated invoice file location",
data: {
invoiceFile: tempPdfLocation,
},
level: "debug",
});
/**
* MIGRATION CODE START - remove when metadata migrated
*/
if (!appConfigV2) {
const migrationService = new ConfigV1ToV2MigrationService(client, authData.saleorApiUrl);
appConfigV2 = await migrationService.migrate();
}
/**
* MIGRATION CODE END
*/
let appConfigV2 =
(await new GetAppConfigurationV2Service({
saleorApiUrl: authData.saleorApiUrl,
apiClient: client,
}).getConfiguration()) ?? new AppConfigV2();
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",
});