add new Logging Guidelines section to README (re. FINERACT-942)
This commit is contained in:
@@ -265,6 +265,25 @@ Governance and Policies
|
||||
documents the process through which you can become a committer in this project.
|
||||
|
||||
|
||||
Error Handling Guidelines
|
||||
------------------
|
||||
* When catching exceptions, either rethrow them, or log them. Either way, include the root cause by using `catch (SomeException e)` and then either `throw AnotherException("..details..", e)` or `LOG.error("...context...", e)`.
|
||||
* Completely empty catch blocks are VERY suspicous! Are you sure that you want to just "swallow" an exception? Really, 100% totally absolutely sure?? ;-) Such "normal exceptions which just happen sometimes but are actually not really errors" are almost always a bad idea, can be a performance issue, and typically are an indication of another problem - e.g. the use of a wrong API which throws an Exception for an expected condition, when really you would want to use another API that instead returns something empty or optional.
|
||||
* In tests, you'll typically never catch exceptions, but just propagate them, with `@Test void testXYZ() throws SomeException, AnotherException`..., so that the test fails if the exception happens. Unless you actually really want to test for the occurence of a problem - in that case, use [JUnit's Assert.assertThrows()](https://github.com/junit-team/junit4/wiki/Exception-testing) (but not `@Test(expected = SomeException.class)`).
|
||||
* Never catch `NullPointerException` & Co.
|
||||
|
||||
Logging Guidelines
|
||||
------------------
|
||||
* We use [SLF4J](http://www.slf4j.org) as our logging API.
|
||||
* Never, ever, use `System.out` and `System.err` or `printStackTrace()` anywhere, but always `LOG.info()` or `LOG.error()` instead.
|
||||
* Use placeholder (`LOG.error("Could not... details: {}", something, exception)`) and never String concatenation (`LOG.error("Could not... details: " + something, exception)`)
|
||||
* Which Log Level is appropriate?
|
||||
* `LOG.error()` should be used to inform an "operator" running Fineract who supervises error logs of an unexpected condition. This includes technical problems with an external "environment" (e.g. can't reach a database), and situations which are likely bugs which need to be fixed in the code. They do NOT include e.g. validation errors for incoming API requests - that is signaled through the API response - and does (should) not be logged as an error. (Note that there is no _FATAL_ level in SLF4J; a "FATAL" event should just be logged as an _ERROR_.)
|
||||
* `LOG.warn()` should be using sparingly. Make up your mind if it's an error (above) - or not!
|
||||
* `LOG.info()` can be used notably for one-time actions taken during start-up. It should typically NOT be used to print out "regular" application usage information. The default logging configuration always outputs the application INFO logs, and in production under load, there's really no point to constantly spew out lots of information from frequently traversed paths in the code about what's going on. (Metrics are a better way.) `LOG.info()` *can* be used freely in tests though.
|
||||
* `LOG.debug()` can be used anywhere in the code to log things that may be useful during investigations of specific problems. They are not shown in the default logging configuration, but can be enabled for troubleshooting. Developers should typically "turn down" most `LOG.info()` which they used while writing a new feature to "follow along what happens during local testing" to `LOG.debug()` for production before we merge their PRs.
|
||||
* `LOG.trace()` is not used in Fineract.
|
||||
|
||||
Pull Requests
|
||||
-------------
|
||||
|
||||
|
||||
Reference in New Issue
Block a user