-
Notifications
You must be signed in to change notification settings - Fork 16
Decode error message and error type #500
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
Conversation
| debug!("Failed to decode error message: {e}"); | ||
| m.to_string() | ||
| } | ||
| }; |
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.
I'm pretty sure there's a cleaner way to do this
| let decoded_message = match base64_to_string(m) { | ||
| Ok(decoded) => decoded, | ||
| Err(e) => { | ||
| debug!("Failed to decode error message: {e}"); |
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.
Message is too ambiguous, since you're actually setting the error at the end, so it's kinda misleading and user might think that it failed, but the error message is actually there
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.
Needs unit testing
|
|
||
| if let Some(m) = message { | ||
| let decoded_message = base64_to_string(m).unwrap_or_else(|_| { | ||
| debug!("Error message header may not be encoded, setting as is"); |
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.
Much more cleaner, great!
| } | ||
| }; | ||
| let decoded_stack = base64_to_string(s).unwrap_or_else(|_| { | ||
| debug!("Error stack header may not be encoded, setting as is"); |
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.
This always comes encoded, so the previous error message was OK
duncanista
left a comment
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.
Let's keep the previous message for the error stack, overall LGTM
## Summary of changes Encodes `error.msg` and `error.type` like the previously encoded `error.stack`.  Related extension changes: DataDog/datadog-agent#32231 DataDog/datadog-lambda-extension#500 ## Reason for change Prevents the tracer from throwing an exception due to non-ASCII characters in these header values. [[SLES-2001] .NET Datadog Tracer throws exception from AWS Lambda Function](https://datadoghq.atlassian.net/browse/SLES-2001) [[SVLS-6041] Having non-ascii characters in a stack trace causes an exception within the tracer.](https://datadoghq.atlassian.net/browse/SVLS-6041) ## Implementation details Encodes the string values to UTF-8 bytes and then to a Base64 string. ## Test coverage Updated tests to check for encoded values. ## Other details <!-- Fixes #{issue} --> <!--⚠️ Note: where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. --> [SLES-2001]: https://datadoghq.atlassian.net/browse/SLES-2001?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [SVLS-6041]: https://datadoghq.atlassian.net/browse/SVLS-6041?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
## Summary of changes Encodes `error.msg` and `error.type` like the previously encoded `error.stack`.  Related extension changes: DataDog/datadog-agent#32231 DataDog/datadog-lambda-extension#500 ## Reason for change Prevents the tracer from throwing an exception due to non-ASCII characters in these header values. [[SLES-2001] .NET Datadog Tracer throws exception from AWS Lambda Function](https://datadoghq.atlassian.net/browse/SLES-2001) [[SVLS-6041] Having non-ascii characters in a stack trace causes an exception within the tracer.](https://datadoghq.atlassian.net/browse/SVLS-6041) ## Implementation details Encodes the string values to UTF-8 bytes and then to a Base64 string. ## Test coverage Updated tests to check for encoded values. ## Other details <!-- Fixes #{issue} --> <!--⚠️ Note: where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. --> [SLES-2001]: https://datadoghq.atlassian.net/browse/SLES-2001?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [SVLS-6041]: https://datadoghq.atlassian.net/browse/SVLS-6041?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
What does this PR do?
Decodes error.msg and error.type headers as was previously done for the error.stack header, because of DataDog/dd-trace-dotnet#6438 encoding error.msg and error.type.
Motivation
[SLES-2001] .NET Datadog Tracer throws exception from AWS Lambda Function
[SVLS-6041] Having non-ascii characters in a stack trace causes an exception within the tracer.
Testing
Tested with a lambda that throws an exception with an error message containing a non-ASCII character.
