-
Notifications
You must be signed in to change notification settings - Fork 375
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
[APPSEC-55194] Handle low-level libddwaf exceptions #4041
[APPSEC-55194] Handle low-level libddwaf exceptions #4041
Conversation
BenchmarksBenchmark execution time: 2024-10-29 10:37:16 Comparing candidate commit a1ebf22 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 metrics, 2 unstable metrics. |
178d1d8
to
2cb89c0
Compare
2cb89c0
to
a1ebf22
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4041 +/- ##
=======================================
Coverage 97.85% 97.85%
=======================================
Files 1321 1321
Lines 79318 79331 +13
Branches 3904 3905 +1
=======================================
+ Hits 77620 77633 +13
Misses 1698 1698 ☔ View full report in Codecov by Sentry. |
expect(telemetry).to receive(:error).with(/libddwaf:[\d.]+ execution error: :err_internal/) | ||
expect(telemetry).to receive(:report) |
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 suppose to be very unfortunate case, we will start with this approach and refactor it next (to report once?)
What does this PR do?
Wrap
Processor::Context#run
with an error handling and telemetry reporting.Motivation:
Is some specific situations, our
libddwaf-rb
is going to throw an exception when it fails to convert C-object into Ruby-object and some other low-level occasions.Change log entry
Not needed.
Additional Notes:
Likely we would change the interface of libddwaf-rb soon™️, but in foreseen future we need to be able to land on feet when something wrong happen with our FFI binding.
How to test the change?
CI is enough.