NPE in GelfAppender.append kills the host app

[mike-hogan]

Using these dependencies:

ch.qos.logback.logback-core 1.1.2
ch.qos.logback.logback-classic 1.1.2
me.moocar.logback-gelf 0.12
com.google.code.gson.gson 2.2.4

I somehow managed to product this stacktrace:

java.lang.NullPointerException
at me.moocar.logbackgelf.GelfAppender.append(GelfAppender.java:60)
at me.moocar.logbackgelf.GelfAppender.append(GelfAppender.java:21)
at ch.qos.logback.core.AppenderBase.doAppend(AppenderBase.java:85)
at ch.qos.logback.core.spi.AppenderAttachableImpl.appendLoopOnAppenders(AppenderAttachableImpl.java:48)
at ch.qos.logback.classic.Logger.appendLoopOnAppenders(Logger.java:273)
at ch.qos.logback.classic.Logger.callAppenders(Logger.java:260)
at ch.qos.logback.classic.Logger.buildLoggingEventAndAppend(Logger.java:442)
at ch.qos.logback.classic.Logger.filterAndLog0Or3Plus(Logger.java:396)
at ch.qos.logback.classic.Logger.debug(Logger.java:503)

And it killed my app.

I can see that the try/catch in GelfAppender.append re-throws the exception. Should it now swallow it?

[mike-hogan]

I figured out how I managed to produce this NPE. I was using an environment variable for the graylog server param, like this

    <graylog2ServerHost>${GRAYLOG_SERVER_IP}</graylog2ServerHost>

and it was not set, so the value GRAYLOG_SERVER_IP_IS_UNDEFINED comes in, which causes this exception

Caused by: java.lang.IllegalStateException: Unknown host: GRAYLOG_SERVER_IP_IS_UNDEFINED: unknown error. Make sure you have specified the 'graylog2ServerHost' property correctly in your logback.xml'
at at me.moocar.logbackgelf.InternetUtils.getInetAddress(InternetUtils.java:53)
at at me.moocar.logbackgelf.GelfAppender.initExecutor(GelfAppender.java:89)

Then subsequent appends throw NPE

[Moocar]

Perhaps a good middle ground here is to add a regex for graylog2ServerHost to ensure that it matches an IP. Then, if it doesn't, print a warning. PRs welcome :) otherwise I'll eventually get to it.

[mike-hogan]

I see. What if somebody wanted to use a dns name? As it happens my plan was to move my system to do that, so apps can keep logging if I cycle my graylog server and it gets a new ip address.

[Moocar]

yeah, good point. I guess this would he hard to protect against. Out of interest, what part of the stack is setting the value to "GRAYLOG_SERVER_IP_IS_UNDEFINED"? I've never seen that before.

[mike-hogan]

I think its logback.

[Moocar]

Closing as error message is better after merging #49