[NO-TICKET] Apply more of standardrb autoformatter to profiler files #3844
Conversation
We were carrying these from the 1.x times, when we still supported Ruby 2.3 (or even 2.1). We can clean them up now!
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
|
|
||
| flush_and_wait | ||
| interrupted = false | ||
| rescue Exception => e # rubocop:disable Lint/RescueException |
There was a problem hiding this comment.
⚪ Code Quality Violation
| rescue Exception => e # rubocop:disable Lint/RescueException | |
| rescue StandardError => e # rubocop:disable Lint/RescueException |
Do not rescue the Exception class (...read more)
The rule "Do not rescue the Exception class" is a crucial practice in Ruby programming for handling exceptions. The Exception class is the root of Ruby's exception hierarchy, so when you rescue Exception, you're potentially catching and handling severe system errors that Ruby itself is trying to bubble up. These could be fundamental issues like memory overflows and syntax errors, which could cause the program to behave unexpectedly or even crash.
Rescuing the Exception class can lead to major problems in debugging since it can hide the true nature of the error and its source. It makes it harder to pinpoint where and why the error occurred. This can lead to significant delays in identifying and resolving coding issues.
Instead of rescuing the Exception class, it is better to rescue more specific error classes or use StandardError which is the superclass for most error types. For instance, if you're expecting possible nil values, use rescue NoMethodError. This allows Ruby to handle severe system errors appropriately and ensures that you're only rescuing the errors you expect. This practice makes your code safer, more predictable, and easier to maintain and debug.
| ) | ||
| end | ||
| Datadog.logger.debug('IdleSamplingHelper thread stopping cleanly') | ||
| rescue Exception => e # rubocop:disable Lint/RescueException |
There was a problem hiding this comment.
⚪ Code Quality Violation
| rescue Exception => e # rubocop:disable Lint/RescueException | |
| rescue StandardError => e # rubocop:disable Lint/RescueException |
Do not rescue the Exception class (...read more)
The rule "Do not rescue the Exception class" is a crucial practice in Ruby programming for handling exceptions. The Exception class is the root of Ruby's exception hierarchy, so when you rescue Exception, you're potentially catching and handling severe system errors that Ruby itself is trying to bubble up. These could be fundamental issues like memory overflows and syntax errors, which could cause the program to behave unexpectedly or even crash.
Rescuing the Exception class can lead to major problems in debugging since it can hide the true nature of the error and its source. It makes it harder to pinpoint where and why the error occurred. This can lead to significant delays in identifying and resolving coding issues.
Instead of rescuing the Exception class, it is better to rescue more specific error classes or use StandardError which is the superclass for most error types. For instance, if you're expecting possible nil values, use rescue NoMethodError. This allows Ruby to handle severe system errors appropriately and ensures that you're only rescuing the errors you expect. This practice makes your code safer, more predictable, and easier to maintain and debug.
| self.class._native_sampling_loop(self) | ||
|
|
||
| Datadog.logger.debug('CpuAndWallTimeWorker thread stopping cleanly') | ||
| rescue Exception => e # rubocop:disable Lint/RescueException |
There was a problem hiding this comment.
⚪ Code Quality Violation
| rescue Exception => e # rubocop:disable Lint/RescueException | |
| rescue StandardError => e # rubocop:disable Lint/RescueException |
Do not rescue the Exception class (...read more)
The rule "Do not rescue the Exception class" is a crucial practice in Ruby programming for handling exceptions. The Exception class is the root of Ruby's exception hierarchy, so when you rescue Exception, you're potentially catching and handling severe system errors that Ruby itself is trying to bubble up. These could be fundamental issues like memory overflows and syntax errors, which could cause the program to behave unexpectedly or even crash.
Rescuing the Exception class can lead to major problems in debugging since it can hide the true nature of the error and its source. It makes it harder to pinpoint where and why the error occurred. This can lead to significant delays in identifying and resolving coding issues.
Instead of rescuing the Exception class, it is better to rescue more specific error classes or use StandardError which is the superclass for most error types. For instance, if you're expecting possible nil values, use rescue NoMethodError. This allows Ruby to handle severe system errors appropriately and ensures that you're only rescuing the errors you expect. This practice makes your code safer, more predictable, and easier to maintain and debug.
|
|
||
| flush_and_wait | ||
| interrupted = false | ||
| rescue Exception => e # rubocop:disable Lint/RescueException |
There was a problem hiding this comment.
⚪ Code Quality Violation
| rescue Exception => e # rubocop:disable Lint/RescueException | |
| rescue StandardError => e # rubocop:disable Lint/RescueException |
Do not rescue the Exception class (...read more)
The rule "Do not rescue the Exception class" is a crucial practice in Ruby programming for handling exceptions. The Exception class is the root of Ruby's exception hierarchy, so when you rescue Exception, you're potentially catching and handling severe system errors that Ruby itself is trying to bubble up. These could be fundamental issues like memory overflows and syntax errors, which could cause the program to behave unexpectedly or even crash.
Rescuing the Exception class can lead to major problems in debugging since it can hide the true nature of the error and its source. It makes it harder to pinpoint where and why the error occurred. This can lead to significant delays in identifying and resolving coding issues.
Instead of rescuing the Exception class, it is better to rescue more specific error classes or use StandardError which is the superclass for most error types. For instance, if you're expecting possible nil values, use rescue NoMethodError. This allows Ruby to handle severe system errors appropriately and ensures that you're only rescuing the errors you expect. This practice makes your code safer, more predictable, and easier to maintain and debug.
| ) | ||
| end | ||
| Datadog.logger.debug('IdleSamplingHelper thread stopping cleanly') | ||
| rescue Exception => e # rubocop:disable Lint/RescueException |
There was a problem hiding this comment.
⚪ Code Quality Violation
| rescue Exception => e # rubocop:disable Lint/RescueException | |
| rescue StandardError => e # rubocop:disable Lint/RescueException |
Do not rescue the Exception class (...read more)
The rule "Do not rescue the Exception class" is a crucial practice in Ruby programming for handling exceptions. The Exception class is the root of Ruby's exception hierarchy, so when you rescue Exception, you're potentially catching and handling severe system errors that Ruby itself is trying to bubble up. These could be fundamental issues like memory overflows and syntax errors, which could cause the program to behave unexpectedly or even crash.
Rescuing the Exception class can lead to major problems in debugging since it can hide the true nature of the error and its source. It makes it harder to pinpoint where and why the error occurred. This can lead to significant delays in identifying and resolving coding issues.
Instead of rescuing the Exception class, it is better to rescue more specific error classes or use StandardError which is the superclass for most error types. For instance, if you're expecting possible nil values, use rescue NoMethodError. This allows Ruby to handle severe system errors appropriately and ensures that you're only rescuing the errors you expect. This practice makes your code safer, more predictable, and easier to maintain and debug.
| self.class._native_sampling_loop(self) | ||
|
|
||
| Datadog.logger.debug('CpuAndWallTimeWorker thread stopping cleanly') | ||
| rescue Exception => e # rubocop:disable Lint/RescueException |
There was a problem hiding this comment.
⚪ Code Quality Violation
| rescue Exception => e # rubocop:disable Lint/RescueException | |
| rescue StandardError => e # rubocop:disable Lint/RescueException |
Do not rescue the Exception class (...read more)
The rule "Do not rescue the Exception class" is a crucial practice in Ruby programming for handling exceptions. The Exception class is the root of Ruby's exception hierarchy, so when you rescue Exception, you're potentially catching and handling severe system errors that Ruby itself is trying to bubble up. These could be fundamental issues like memory overflows and syntax errors, which could cause the program to behave unexpectedly or even crash.
Rescuing the Exception class can lead to major problems in debugging since it can hide the true nature of the error and its source. It makes it harder to pinpoint where and why the error occurred. This can lead to significant delays in identifying and resolving coding issues.
Instead of rescuing the Exception class, it is better to rescue more specific error classes or use StandardError which is the superclass for most error types. For instance, if you're expecting possible nil values, use rescue NoMethodError. This allows Ruby to handle severe system errors appropriately and ensures that you're only rescuing the errors you expect. This practice makes your code safer, more predictable, and easier to maintain and debug.
BenchmarksBenchmark execution time: 2024-08-14 15:01:29 Comparing candidate commit 52c3b5a in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 21 metrics, 2 unstable metrics. scenario:profiler - sample timeline=false
scenario:profiler - sample+serialize retain_every=10 heap_samples=false heap_size=false heap_sample_every=1 skip_end_gc=false
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3844 +/- ##
==========================================
- Coverage 97.85% 97.83% -0.02%
==========================================
Files 1264 1264
Lines 75737 75725 -12
Branches 3729 3729
==========================================
- Hits 74110 74085 -25
- Misses 1627 1640 +13 ☔ View full report in Codecov by Sentry. |
Yes: The autofix was only for quotes in symbols not strings, I'll open up the bigger "make all strings consistent" next :) |
What does this PR do?
In #3801 we adopted standardrb for the profiler. At the time, I left a few cops disabled:
This is the first of those smaller PRs that can be reviewed sanely.
Motivation:
Fully adopt standardrb for profiling.
Additional Notes:
The autofix to remove begin/end creates a lot of diff noise. I recommend reviewing this PR without whitespace as it's easier to spot the change.
How to test the change?
This PR does not change any behaviors, so existing test coverage being green is the thing to aim for :)