-
Notifications
You must be signed in to change notification settings - Fork 0
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
base: main
Are you sure you want to change the base?
Changes from all commits
379480b
bcb408f
a125834
1159a6b
89bd308
c3f5c24
cc23d81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
} | ||
|
||
try | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -261,6 +261,8 @@ invoice is | |
|
||
private async Task<bool> AttemptToPayInvoiceWithBraintreeAsync(Invoice invoice, Customer customer) | ||
{ | ||
LogBraintreeConfiguration(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
{ | ||
|
@@ -404,4 +406,48 @@ private async Task<bool> AttemptToPayInvoiceWithStripeAsync(Invoice invoice) | |
throw; | ||
} | ||
} | ||
|
||
private void LogBraintreeConfiguration() | ||
{ | ||
var environment = System.Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
} | ||
} | ||
} |
There was a problem hiding this comment.
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.