This is an automated email from the ASF dual-hosted git repository.
vorburger pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/fineract.git
The following commit(s) were added to refs/heads/develop by this push:
new 4194da2 add new Logging Guidelines section to README (re.
FINERACT-942)
4194da2 is described below
commit 4194da2b90479f63e6a8ead2b2dd6d47092c1657
Author: Michael Vorburger ⛑️ <[email protected]>
AuthorDate: Tue May 19 23:28:04 2020 +0200
add new Logging Guidelines section to README (re. FINERACT-942)
---
README.md | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/README.md b/README.md
index 988b9c4..9f7d744 100644
--- a/README.md
+++ b/README.md
@@ -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 inste [...]
+* 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_ le [...]
+ * `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
-------------
