Skip to content
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

Appender [SENTRY] failed to append. java.lang.NullPointerException #1361

Closed
1 task done
donbeave opened this issue Mar 29, 2021 · 2 comments
Closed
1 task done

Appender [SENTRY] failed to append. java.lang.NullPointerException #1361

donbeave opened this issue Mar 29, 2021 · 2 comments
Labels

Comments

@donbeave
Copy link
Contributor

Platform:

  • Java

The version of the SDK:
4.3.0


I'm using the latest version of sentry logback library and found one issue with it. In my case sometimes MDC can have a keys with null values, which affects Sentry with such exception:

06:31:30,919 |-ERROR in io.sentry.logback.SentryAppender[SENTRY] - Appender [SENTRY] failed to append. java.lang.NullPointerException
	at java.lang.NullPointerException
	at 	at java.base/java.util.concurrent.ConcurrentHashMap.putVal(ConcurrentHashMap.java:1011)
	at 	at java.base/java.util.concurrent.ConcurrentHashMap.putAll(ConcurrentHashMap.java:1089)
	at 	at java.base/java.util.concurrent.ConcurrentHashMap.<init>(ConcurrentHashMap.java:852)
	at 	at io.sentry.util.CollectionUtils.shallowCopy(CollectionUtils.java:42)
	at 	at io.sentry.logback.SentryAppender.createEvent(SentryAppender.java:95)
	at 	at io.sentry.logback.SentryAppender.append(SentryAppender.java:60)
	at 	at io.sentry.logback.SentryAppender.append(SentryAppender.java:29)
	at 	at ch.qos.logback.core.UnsynchronizedAppenderBase.doAppend(UnsynchronizedAppenderBase.java:84)
	at 	at ch.qos.logback.core.spi.AppenderAttachableImpl.appendLoopOnAppenders(AppenderAttachableImpl.java:51)
	at 	at ch.qos.logback.classic.Logger.appendLoopOnAppenders(Logger.java:270)
	at 	at ch.qos.logback.classic.Logger.callAppenders(Logger.java:257)
	at 	at ch.qos.logback.classic.Logger.buildLoggingEventAndAppend(Logger.java:421)
	at 	at ch.qos.logback.classic.Logger.filterAndLog_0_Or3Plus(Logger.java:383)
	at 	at ch.qos.logback.classic.Logger.log(Logger.java:765)
	at 	at org.slf4j.bridge.SLF4JBridgeHandler.callLocationAwareLogger(SLF4JBridgeHandler.java:221)
	at 	at org.slf4j.bridge.SLF4JBridgeHandler.publish(SLF4JBridgeHandler.java:303)
	at 	at java.logging/java.util.logging.Logger.log(Logger.java:979)
	at 	at java.logging/java.util.logging.Logger.doLog(Logger.java:1006)
	at 	at java.logging/java.util.logging.Logger.log(Logger.java:1117)

Steps to reproduce:

MDC.put("user-id", null);

// this error log will not send to Sentry, because it will fail with the NPE
log.error("Test");

How to fix:

The NPE was thrown by ConcurrentHashMap, which is not supporting null values. This implementation is used in CollectionUtils.java:

  public static <K, V> @Nullable Map<K, V> shallowCopy(@Nullable Map<K, V> map) {
    if (map != null) {
      return new ConcurrentHashMap<>(map);
    } else {
      return null;
    }
  }

I think it's better to replace the usage of ConcurrentHashMap with HashMap, because MDC API is allowing to use null values.

@maciejwalkowiak @marandaneto @bruno-garcia what do you think?

@bruno-garcia
Copy link
Member

If the map is on the scope it must be thread-safe. if it's on the Event only it could be a HashMap.
Sentry won't really be useful for a key with no value though so it sounds like it makes more sense to not add it at all if the value is null.

@maciejwalkowiak
Copy link
Contributor

Fixed in #1364

@stephanie-anderson stephanie-anderson removed the Type: Bug Something isn't working label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants