-
-
Notifications
You must be signed in to change notification settings - Fork 226
Feat: allow logcat attachment to be previewed in Sentry #3711
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
|
eb64624 to
3106b92
Compare
|
I can't find a way to get But it's not like we need to test this since the |
@bricefriha the device tests are just unit tests that get run on Android or iOS by our device runner here: sentry-dotnet/test/Sentry.Maui.Device.TestApp/Startup.cs Lines 22 to 29 in a2bdc82
So you don't need to do anything special... just write some unit tests for Note: I see that method isn't very testable... it references static methods like |
67cdfc8 to
defbf1a
Compare
Co-authored-by: James Crosswell <[email protected]>
Co-authored-by: James Crosswell <[email protected]>
Co-authored-by: James Crosswell <[email protected]>
jamescrosswell
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.
Looks good - thanks @bricefriha !
Just one question - have you actually tested this in an android app to make sure the logs come through OK and can be previewed in Sentry?
@jamescrosswell I have tested with the sample we have and it can be previewed in Sentry (as you can see on the screenshot in the description) |
Context
As issue #3703 reports, Logcats files currently attached to Sentry issues are not previewable.
This issue is due to the
mime-typesent with the file, as it currently has its own type,text/logcat. However, Sentry's preview component does not support this type.This PR aims to tackle just that by changing the mime-type we send from
text/logcattotext/plain.Caviat
I personally believe that the user should have a more console-like experience when looking at the preview on Sentry. To tackle this, an issue in Sentry's repo is currently open. So, the PR would offer more of a temporary fix than a complete solution.
Therefore, when/if
text/logcatgets support, we could change the mime-type back on our end