Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-7452] Resolve PayPal issue #25

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
12 changes: 10 additions & 2 deletions src/Billing/Services/Implementations/InvoiceCreatedHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ public async Task HandleAsync(Event parsedEvent)
}
catch (Exception exception)
{
logger.LogError(exception, "Failed to attempt paying for invoice while handling 'invoice.created' event ({EventID})", parsedEvent.Id);
logger.LogError(exception, "Failed to attempt paying for invoice while handling 'invoice.created' event ({EventID}) | Message: {Message}",
parsedEvent.Id,
exception.Message);

throw;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Re-throwing the exception after logging. Ensure this change in behavior is intended and handled appropriately by the caller.

}

try
Expand All @@ -58,7 +62,11 @@ public async Task HandleAsync(Event parsedEvent)
}
catch (Exception exception)
{
logger.LogError(exception, "Failed to record provider invoice line items while handling 'invoice.created' event ({EventID})", parsedEvent.Id);
logger.LogError(exception, "Failed to record provider invoice line items while handling 'invoice.created' event ({EventID}) | Message: {Message}",
parsedEvent.Id,
exception.Message);

throw;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Another instance of re-throwing the exception. Verify that this is the desired behavior.

}
}
}
46 changes: 46 additions & 0 deletions src/Billing/Services/Implementations/StripeEventUtilityService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ invoice is

private async Task<bool> AttemptToPayInvoiceWithBraintreeAsync(Invoice invoice, Customer customer)
{
LogBraintreeConfiguration();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider moving this method call to the constructor to ensure logging happens only once per service instance


_logger.LogDebug("Attempting to pay invoice with Braintree");
if (!customer?.Metadata?.ContainsKey("btCustomerId") ?? true)
{
Expand Down Expand Up @@ -404,4 +406,48 @@ private async Task<bool> AttemptToPayInvoiceWithStripeAsync(Invoice invoice)
throw;
}
}

private void LogBraintreeConfiguration()
{
var environment = System.Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Use IHostEnvironment instead of Environment.GetEnvironmentVariable for consistency with ASP.NET Core practices


if (environment != "QA")
{
_logger.LogInformation("Braintree: Not logging for {Environment}", environment);
return;
}

var merchantId = _globalSettings.Braintree.MerchantId;

if (string.IsNullOrEmpty(merchantId))
{
_logger.LogWarning("Braintree Merchant ID: null or empty");
}
else
{
_logger.LogInformation("Braintree Merchant ID: {MerchantId}", merchantId[..5]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Logging even partial keys could be a security risk. Consider using a boolean flag instead

}

var publicKey = _globalSettings.Braintree.PublicKey;

if (string.IsNullOrEmpty(publicKey))
{
_logger.LogWarning("Braintree Public Key: null or empty");
}
else
{
_logger.LogInformation("Braintree Public Key: {PublicKey}", publicKey[..5]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Logging even partial keys could be a security risk. Consider using a boolean flag instead

}

var privateKey = _globalSettings.Braintree.PrivateKey;

if (string.IsNullOrEmpty(privateKey))
{
_logger.LogWarning("Braintree Private Key: null or empty");
}
else
{
_logger.LogInformation("Braintree Private Key: {PrivateKey}", privateKey[..5]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Logging even partial keys could be a security risk. Consider using a boolean flag instead

}
}
}
Loading