defuse / php-encryption

Simple Encryption in PHP.
MIT License
3.78k stars 308 forks source link

Runtime tests: Exceptions showing in transaction tracing #485

Closed MarkG-IEL closed 1 year ago

MarkG-IEL commented 3 years ago

Not sure if there is any desire to change anything, but I thought it worthwhile to mention this:

When doing transaction tracing on NewRelic, all the exceptions thrown in RuntimeTests::runtimeTest(); are recorded as errors in the trace. While NR usually only displays un-caught errors, tracing will show all of them. While it's possible to tell NR to ignore that exception class, that also means ignoring real issues that throw it. This is a case where these false positives are going to wind up creating error fatigue, and real problems will go unnoticed.

I appreciate that this is a security-focused library that does runtime checking. I don't see a path forward that would keep valid runtime tests while not throwing exceptions, or even a different class of exceptions as both would not be a true test of the code. Any thoughts?

Screenshot 2021-05-17 165200

defuse commented 3 years ago

IMO, I could be wrong, but exception throws and catches should be considered a normal part of any program's execution. If something is logging every exception that gets thrown, regardless of whether it's caught or not, it's to be expected that its output will be verbose and noisy. I don't think it's reasonable to write code in such a way that exceptions are only ever thrown (regardless of being caught) when there's a loggable event.