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

Catch process.TotalProcessorTime exception #44

Merged
merged 4 commits into from
Sep 24, 2024

Conversation

timmac-qmc
Copy link
Contributor

process.TotalProcessorTime is not supported on all platforms
image

process.TotalProcessorTime is not supported on all platforms
@konraddysput
Copy link
Collaborator

Thanks for reaching us to us with pull request!

Calculating process.age can be done via internal calculation. The catch block supposes to catch other exceptions related to memory information. @perf2711 can you share your approach?

@timmac-qmc
Copy link
Contributor Author

The catch block supposes to catch other exceptions related to memory information.

The code seems to contradict this statement considering you're explicitly catching "PlatformNotSupportedException"

@konraddysput konraddysput added the bug Something isn't working label Sep 9, 2024
Copy link
Collaborator

@perf2711 perf2711 left a comment

Choose a reason for hiding this comment

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

Thank you for your submission!

This change is sound. The only thing I could suggest is that we keep try/catch separate, so we can still try and retrieve other stuff.

As for the alternative approach, we can implement this after this is merged, so as not to block this change.

Backtrace/Model/JsonData/BacktraceAttributes.cs Outdated Show resolved Hide resolved
@perf2711
Copy link
Collaborator

@timmac-qmc can you fix formatting issues, so we can merge this change? See this check.

@timmac-qmc
Copy link
Contributor Author

@timmac-qmc can you fix formatting issues, so we can merge this change? See this check.

Should hopefully be correct now

@timmac-qmc
Copy link
Contributor Author

timmac-qmc commented Sep 19, 2024

Found some whitespace, have to say that failing on that seems a bit extreme, it has literally no effect on formatting...

Remove whitespace (that's not even an issue...)
@perf2711 perf2711 merged commit 8b34165 into backtrace-labs:master Sep 24, 2024
2 checks passed
@timmac-qmc timmac-qmc deleted the patch-1 branch September 27, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants