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

NPE when encoding and decoding nested case class #561

Open
marcoy opened this issue Jan 31, 2017 · 13 comments
Open

NPE when encoding and decoding nested case class #561

marcoy opened this issue Jan 31, 2017 · 13 comments

Comments

@marcoy
Copy link

@marcoy marcoy commented Jan 31, 2017

Hi, I have a case class where one of the fields is referencing another object of the same type.
When I try to decode[NestedClass](str), it throws an NPE. Same when I try to encode to Json.
I'm using circe version 0.7.0 with scala 2.12.

Here is an simple example:

import io.circe._
import io.circe.parser._
import io.circe.syntax._

object NestedExample {
  case class NestedClass(id: Long, displayName: String, nesting: Option[NestedClass])

  implicit val nestedClassDecoder: Decoder[NestedClass] =
    Decoder.forProduct3("id", "display_name", "nesting")(NestedClass.apply)

  implicit val nestedClassEncoder: Encoder[NestedClass] =
    Encoder.forProduct3("id", "display_name", "nesting") { nc =>
      (nc.id, nc.displayName, nc.nesting)
    }

  def main(args: Array[String]): Unit = {
    val testString =
      """
        |{
        |  "id": 10000,
        |  "display_name": "foobar",
        |  "nesting": {
        |    "id": 10001,
        |    "display_name": "quux"
        |  }
        |}
      """.stripMargin

    val errorOrNesting = decode[NestedClass](testString) // NPE

    val inner: NestedClass = NestedClass(10001L, "inner", None)
    val outer: NestedClass = NestedClass(10000L, "outer", Some(inner))
    println(outer.asJson.spaces2) // NPE
  }
}

Stacktrace when decoding

Exception in thread "main" java.lang.NullPointerException
	at io.circe.Decoder$.$anonfun$decodeOption$1(Decoder.scala:623)
	at io.circe.Decoder$$anon$27.tryDecode(Decoder.scala:312)
	at io.circe.ACursor.as(ACursor.scala:336)
	at io.circe.ACursor.get(ACursor.scala:343)
	at io.circe.ProductDecoders$$anon$3.apply(ProductDecoders.scala:36)
	at io.circe.Decoder.decodeJson(Decoder.scala:48)
	at io.circe.Decoder.decodeJson$(Decoder.scala:48)
	at io.circe.ProductDecoders$$anon$3.decodeJson(ProductDecoders.scala:35)
	at io.circe.Parser.finishDecode(Parser.scala:11)
	at io.circe.Parser.finishDecode$(Parser.scala:8)
	at io.circe.parser.package$.finishDecode(package.scala:5)
	at io.circe.Parser.decode(Parser.scala:25)
	at io.circe.Parser.decode$(Parser.scala:24)
	at io.circe.parser.package$.decode(package.scala:5)
	at com.marcoy.circe.nested.NestedExample$.main(NestedExample.scala:32)
	at com.marcoy.circe.nested.NestedExample.main(NestedExample.scala)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

Stacktrace when encoding

Exception in thread "main" java.lang.NullPointerException
	at io.circe.Encoder$$anon$28.apply(Encoder.scala:215)
	at io.circe.Encoder$$anon$28.apply(Encoder.scala:213)
	at io.circe.ProductEncoders$$anon$3.encodeObject(ProductEncoders.scala:38)
	at io.circe.ObjectEncoder.apply(ObjectEncoder.scala:13)
	at io.circe.ObjectEncoder.apply$(ObjectEncoder.scala:13)
	at io.circe.ProductEncoders$$anon$3.apply(ProductEncoders.scala:35)
	at io.circe.syntax.package$EncoderOps$.asJson$extension(package.scala:8)
	at com.marcoy.circe.nested.NestedExample$.main(NestedExample.scala:36)
	at com.marcoy.circe.nested.NestedExample.main(NestedExample.scala)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)
@travisbrown
Copy link
Member

@travisbrown travisbrown commented Jan 31, 2017

Hmm, this is unfortunate. I'm afraid for right now the only option is to use the more explicit approach to defining these instances:

import cats.instances.either._
import cats.syntax.cartesian._
import io.circe._, io.circe.parser._, io.circe.syntax._

case class NestedClass(id: Long, displayName: String, nesting: Option[NestedClass])

object NestedClass {
  implicit val nestedClassDecoder: Decoder[NestedClass] = Decoder.instance { c =>
    (
      c.get[Long]("id") |@|
      c.get[String]("display_name") |@|
      c.get[Option[NestedClass]]("nesting")
    ).map(NestedClass.apply)
  }

  implicit val nestedClassEncoder: Encoder[NestedClass] = Encoder.instance {
    case NestedClass(i, d, n) => Json.obj(
      "id"           -> i.asJson,
      "display_name" -> d.asJson,
      "nesting"      -> n.asJson
    )
  }
}

Or you could use generic-extras:

import io.circe.generic.extras._, io.circe.generic.extras.auto._
import io.circe.parser._, io.circe.syntax._

implicit val config: Configuration = Configuration.default.withSnakeCaseKeys

case class NestedClass(id: Long, displayName: String, nesting: Option[NestedClass])

And then:

scala> val inner: NestedClass = NestedClass(10001L, "inner", None)
inner: NestedClass = NestedClass(10001,inner,None)

scala> val outer: NestedClass = NestedClass(10000L, "outer", Some(inner))
outer: NestedClass = NestedClass(10000,outer,Some(NestedClass(10001,inner,None)))

scala> val json = outer.asJson.spaces2
json: String =
{
  "id" : 10000,
  "display_name" : "outer",
  "nesting" : {
    "id" : 10001,
    "display_name" : "inner",
    "nesting" : null
  }
}

scala> decode[NestedClass](json)
res0: Either[io.circe.Error,NestedClass] = Right(NestedClass(10000,outer,Some(NestedClass(10001,inner,None))))

Neither of these approaches requires the same kind of recursive reference to the current encoders or decoders that you see with forProductN.

For what it's worth you get the same thing with the equivalent method in Argonaut:

import argonaut._, Argonaut._

case class NestedClass(id: Long, displayName: String, nesting: Option[NestedClass])

implicit val decodeNestedClass: DecodeJson[NestedClass] =
  jdecode3L(NestedClass.apply)("id", "display_name", "nesting")

val testString =
      """
        |{
        |  "id": 10000,
        |  "display_name": "foobar",
        |  "nesting": {
        |    "id": 10001,
        |    "display_name": "quux"
        |  }
        |}
      """.stripMargin

val result = Parse.decode[NestedClass](testString)

This was reported as an issue there a couple of years ago, but closed as "not an Argonaut issue per se". I'm not sure we'll be able to do better than that, but at the very least we'll get these methods documented so that it's clear they don't work for recursive case classes.

Thanks for the report, @marcoy, and apologies for the inconvenience.

@marcoy
Copy link
Author

@marcoy marcoy commented Jan 31, 2017

No worries, the two alternatives you outlined are not that bad at all, Thanks! In fact, I didn't know about the .instance(...) and the configuration for snake case.

I'm closing this. Thank you for the help.

@marcoy marcoy closed this Jan 31, 2017
@travisbrown
Copy link
Member

@travisbrown travisbrown commented Feb 1, 2017

Do you mind reopening this, @marcoy? I'd like to have a reminder to take a look at it, and to add some documentation if we decide a fix isn't worth it. I can also open a new issue if you'd prefer.

@marcoy
Copy link
Author

@marcoy marcoy commented Feb 1, 2017

Sure thing.

@wsargent
Copy link

@wsargent wsargent commented Aug 25, 2017

This is an issue when writing an Encoder[Throwable] because of getCause:

implicit val throwableEncoder: Encoder[Throwable] = Encoder.instance { throwable =>
  import io.circe._, io.circe.syntax._

  val message = Option(throwable.getMessage).getOrElse("")
  val elements = Option(throwable.getStackTrace).map(_.toSeq).getOrElse(Seq.empty)
  val declaringClass = throwable.getClass.getName

  Json.obj(
    "message" -> message.asJson,
    "@class" -> declaringClass.asJson,
    "elements" -> elements.asJson,
    "cause" -> Option(throwable.getCause).asJson
  )
}

implicit val stackTraceElementEncoder: Encoder[StackTraceElement] = Encoder.instance { element =>
  import io.circe._, io.circe.syntax._

  val filename = Option(element.getFileName)
  val lineNumber = element.getLineNumber
  val methodName = element.getMethodName
  val className = element.getClassName

  Json.obj(
    "fileName" -> filename.asJson,
    "lineNumber" -> lineNumber.asJson,
    "methodName" -> methodName.asJson,
    "className" -> className.asJson
  )
}
@wsargent
Copy link

@wsargent wsargent commented Aug 25, 2017

Nope, there's something odder going on. Any encoder that uses Throwable or a subclass there of will give the same exception:

  implicit val healthCheckResultEncoder: Encoder[HealthCheck.Result] =
    Encoder.forProduct5("status", "message", "tags", "throwable", "creationTime") { u =>
      val status = u.status match {
        case HealthCheck.Status.Healthy     => "healthy"
        case HealthCheck.Status.Unhealthy   => "unhealthy"
        case HealthCheck.Status.Maintenance => "maintenance"
        case HealthCheck.Status.Zombie      => "zombie"
      }

      val message      = u.message
      val tags         = u.tags
      val throwable: Option[IllegalArgumentException] =    Some(new IllegalArgumentException)
      val creationTime = u.creationTime

      (status, message, tags, throwable, creationTime)
    }

  implicit val throwableEncoder: Encoder[IllegalArgumentException] = Encoder.instance { throwable =>
    import io.circe._, io.circe.syntax._

    val message = "derp" // Option(throwable.getMessage)
    Json.obj(
      "message" -> message.asJson
    )
  }

results in:

[info] java.lang.NullPointerException: null
[info] 	at io.circe.Encoder$$anon$28.apply(Encoder.scala:219)
[info] 	at io.circe.Encoder$$anon$28.apply(Encoder.scala:217)
[info] 	at io.circe.ProductEncoders$$anon$5.encodeObject(ProductEncoders.scala:62)
[info] 	at io.circe.ObjectEncoder.apply(ObjectEncoder.scala:13)
[info] 	at io.circe.ObjectEncoder.apply$(ObjectEncoder.scala:13)
[info] 	at io.circe.ProductEncoders$$anon$5.apply(ProductEncoders.scala:59)
[info] 	at io.circe.Encoder$$anon$7.$anonfun$encodeObject$1(Encoder.scala:290)
[info] 	at scala.collection.TraversableLike.$anonfun$map$1(TraversableLike.scala:234)
[info] 	at scala.collection.immutable.Map$Map1.foreach(Map.scala:120)
[info] 	at scala.collection.TraversableLike.map(TraversableLike.scala:234)
@travisbrown
Copy link
Member

@travisbrown travisbrown commented Aug 25, 2017

@wsargent That shouldn't fail—you're getting NPEs from that code specifically?

@wsargent
Copy link

@wsargent wsargent commented Aug 25, 2017

If I comment out the throwable, it works fine. Add it back in, and boom.

@travisbrown
Copy link
Member

@travisbrown travisbrown commented Aug 25, 2017

Which throwable?

@wsargent
Copy link

@wsargent wsargent commented Aug 25, 2017

Well, the original throwable is

    HealthCheckService(system).register("database", HealthCheck {
      //HealthCheck.Result.healthy("Database connection is successful!")
      throw new IllegalStateException("BLLALAGH")
    })

and when I don't include it in the encoding then everything works:

implicit val healthCheckResultEncoder: Encoder[HealthCheck.Result] =
    Encoder.forProduct4("status", "message", "tags", "creationTime") { u =>
      val status = u.status match {
        case HealthCheck.Status.Healthy     => "healthy"
        case HealthCheck.Status.Unhealthy   => "unhealthy"
        case HealthCheck.Status.Maintenance => "maintenance"
        case HealthCheck.Status.Zombie      => "zombie"
      }

      val message      = u.message
      val tags         = u.tags
       val creationTime = u.creationTime

      (status, message, tags, creationTime)
    }

I am putting together a test project to better isolate this.

@wsargent
Copy link

@wsargent wsargent commented Aug 25, 2017

Can't replicate in a subproject: it won't even compile:

import io.circe._, io.circe.syntax._

object Main {

  def main(args: Array[String]): Unit = {
    implicit val throwableEncoder: Encoder[Throwable] = Encoder.instance { throwable =>
      val maybeMessage = Option(throwable.getMessage)
      val maybeThrowable = Option(throwable.getCause)

      Json.obj(
        "message" -> maybeMessage.asJson,
        "cause" -> maybeThrowable.asJson
      )
    }

    try {
      throw new IllegalStateException("whoops")
    } catch {
      case ex: Throwable =>
        val json = ex.asJson
        println(json.noSpaces)
    }
  }
}
Error:(12, 35) forward reference extends over definition of value throwableEncoder
        "cause" -> maybeThrowable.asJson

The subtyping / variance is off as well -- if I want to work with Exception, I have to do this:

import io.circe._, io.circe.syntax._

object Main {

  def main(args: Array[String]): Unit = {
    implicit val throwableEncoder: Encoder[Exception] = Encoder.instance { throwable =>
      val maybeMessage = Option(throwable.getMessage)
      val maybeThrowable = Option(throwable.getCause)

      Json.obj(
        "message" -> maybeMessage.asJson,
        //"cause" -> maybeThrowable.asJson
      )
    }

    try {
      throw new IllegalStateException("whoops")
    } catch {
      case ex: Exception =>
        val json = ex.asJson
        println(json.noSpaces)
    }
  }

}
@wsargent
Copy link

@wsargent wsargent commented Aug 25, 2017

Okay, it looks like the issue is related to defining the implicit for Encoder[Throwable] after the healthcheckResultEncoder. It works fine when I do "stacktraceEncoder / throwableEncoder / healthcheckEncoder". Still very strange though.

@mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Sep 13, 2017

I also got bit by this, getting a

scala> 
java.lang.NullPointerException
  at io.circe.SeqDecoder.apply(SeqDecoder.scala:18)
  at io.circe.Decoder$$anon$16.apply(Decoder.scala:111)
  at io.circe.Decoder$class.tryDecode(Decoder.scala:33)
  at io.circe.Decoder$$anon$16.tryDecode(Decoder.scala:110)
  at io.circe.ACursor.as(ACursor.scala:336)
  at io.circe.ACursor.get(ACursor.scala:343)
  at io.circe.ProductDecoders$$anon$1.apply(ProductDecoders.scala:12)
  at io.circe.Json.as(Json.scala:97)

The solution (of making an explicit decoder) works for me as well, instead of usingDecoder.forProduct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.