search

com-lihaoyi / upickle

uPickle: a simple, fast, dependency-free JSON & Binary (MessagePack) serialization library for Scala
https://com-lihaoyi.github.io/upickle
MIT License
707 stars 158 forks source link

Improve error reporting #567

Closed aboisvert closed 3 months ago

aboisvert commented 3 months ago

When upickle fails to read to a json document into an object tree, the errors don't provide much context into where in the json/class trees the error happens.

Example:

upickle.core.Abort: expected sequence got string
    upickle.core.SimpleVisitor.visitString(SimpleVisitor.scala:14)
    upickle.core.SimpleVisitor.visitString$(SimpleVisitor.scala:7)
    upickle.implicits.Readers$$anon$19.visitString(Readers.scala:314)
    ujson.Value$.transform(Value.scala:163)
    ujson.Value$.transform(Value.scala:158)
    ujson.AstTransformer.transformObject$$anonfun$1(AstTransformer.scala:21)
    scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
    scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
    scala.collection.IterableOnceOps.foreach(IterableOnce.scala:575)
    scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:573)
    upickle.core.LinkedHashMap.foreach(LinkedHashMap.scala:12)
    ujson.AstTransformer.transformObject(AstTransformer.scala:22)
    ujson.AstTransformer.transformObject$(AstTransformer.scala:8)
    ujson.Value$.transformObject(Value.scala:126)
    ujson.Value$.transform(Value.scala:166)
    ujson.Value.transform(Value.scala:110)
    ujson.Value.transform$(Value.scala:9)
    ujson.Obj.transform(Value.scala:207)
    upickle.Api.read$$anonfun$1(Api.scala:42)
    upickle.core.TraceVisitor$.withTrace(TraceVisitor.scala:18)
    upickle.Api.read(Api.scala:42)
    upickle.Api.read$(Api.scala:16)
    upickle.default$.read(Api.scala:238)

This is bothersome for large documents and complex class hierarchies because it doesn't provide any insight into where the issue lies, even if the document passes json-schema validation, for example.

Are there plans to improve this? Or maybe some configuration option that I missed? Thanks!

aboisvert commented 3 months ago

Of course the moment I file an issue, I discover there's an optional trace argument to upickle.default.read. D'oh.

lihaoyi commented 3 months ago

What API are you using? It's meant to give you the character offset that the failure occurred even without trace. Trace is optional only if you want the pretty JSON path


@ upickle.default.read[Map[String, Seq[String]]]("""{"hello": "world"}""")
upickle.core.AbortException: expected sequence got string at index 10
  ujson.CharParser$$anonfun$reject$1.applyOrElse(CharParser.scala:344)
  ujson.CharParser$$anonfun$reject$1.applyOrElse(CharParser.scala:342)
  scala.runtime.AbstractPartialFunction.apply(AbstractPartialFunction.scala:35)
  ujson.CharParser.parseNested(CharParser.scala:379)
  ujson.CharParser.parseTopLevel0(CharParser.scala:324)
  ujson.CharParser.parseTopLevel(CharParser.scala:308)
  ujson.CharParser.parse(CharParser.scala:59)
  ujson.StringParser$.transform(StringParser.scala:28)
  ujson.StringParser$.transform(StringParser.scala:27)
  ujson.Readable$fromTransformer.transform(Readable.scala:13)
  upickle.Api.$anonfun$read$1(Api.scala:37)
  upickle.core.TraceVisitor$.withTrace(TraceVisitor.scala:18)
  upickle.Api.read(Api.scala:37)
  upickle.Api.read$(Api.scala:36)
  upickle.default$.read(Api.scala:157)
  ammonite.$sess.cmd3$.<clinit>(cmd3.sc:1)
upickle.core.Abort: expected sequence got string
  upickle.core.SimpleVisitor.visitString(SimpleVisitor.scala:14)
  upickle.core.SimpleVisitor.visitString$(SimpleVisitor.scala:13)
  upickle.implicits.Readers$$anon$24.visitString(Readers.scala:280)
  ujson.CharParser.visitString(CharParser.scala:617)
  ujson.CharParser.parseStringValue(CharParser.scala:586)
  ujson.CharParser.parseNested(CharParser.scala:379)
  ujson.CharParser.parseTopLevel0(CharParser.scala:324)
  ujson.CharParser.parseTopLevel(CharParser.scala:308)
  ujson.CharParser.parse(CharParser.scala:59)
  ujson.StringParser$.transform(StringParser.scala:28)
  ujson.StringParser$.transform(StringParser.scala:27)
  ujson.Readable$fromTransformer.transform(Readable.scala:13)
  upickle.Api.$anonfun$read$1(Api.scala:37)
  upickle.core.TraceVisitor$.withTrace(TraceVisitor.scala:18)
  upickle.Api.read(Api.scala:37)
  upickle.Api.read$(Api.scala:36)
  upickle.default$.read(Api.scala:157)
  ammonite.$sess.cmd3$.<clinit>(cmd3.sc:1)

@ upickle.default.read[Map[String, Seq[String]]]("""{"hello": "world"}""", trace = true)
upickle.core.TraceVisitor$TraceException: $['hello']

upickle.core.AbortException: expected sequence got string at index 10
  ujson.CharParser$$anonfun$reject$1.applyOrElse(CharParser.scala:344)
  ujson.CharParser$$anonfun$reject$1.applyOrElse(CharParser.scala:342)
  scala.runtime.AbstractPartialFunction.apply(AbstractPartialFunction.scala:35)
  ujson.CharParser.parseNested(CharParser.scala:379)
  ujson.CharParser.parseTopLevel0(CharParser.scala:324)
  ujson.CharParser.parseTopLevel(CharParser.scala:308)
  ujson.CharParser.parse(CharParser.scala:59)
  ujson.StringParser$.transform(StringParser.scala:28)
  ujson.StringParser$.transform(StringParser.scala:27)
  ujson.Readable$fromTransformer.transform(Readable.scala:13)
  upickle.Api.$anonfun$read$1(Api.scala:37)
  upickle.core.TraceVisitor$.withTrace(TraceVisitor.scala:21)
  upickle.Api.read(Api.scala:37)
  upickle.Api.read$(Api.scala:36)
  upickle.default$.read(Api.scala:157)
  ammonite.$sess.cmd4$.<clinit>(cmd4.sc:1)
upickle.core.Abort: expected sequence got string
  upickle.core.SimpleVisitor.visitString(SimpleVisitor.scala:14)
  upickle.core.SimpleVisitor.visitString$(SimpleVisitor.scala:13)
  upickle.implicits.Readers$$anon$24.visitString(Readers.scala:280)
  upickle.core.Visitor$Delegate.visitString(Visitor.scala:156)
  ujson.CharParser.visitString(CharParser.scala:617)
  ujson.CharParser.parseStringValue(CharParser.scala:586)
  ujson.CharParser.parseNested(CharParser.scala:379)
  ujson.CharParser.parseTopLevel0(CharParser.scala:324)
  ujson.CharParser.parseTopLevel(CharParser.scala:308)
  ujson.CharParser.parse(CharParser.scala:59)
  ujson.StringParser$.transform(StringParser.scala:28)
  ujson.StringParser$.transform(StringParser.scala:27)
  ujson.Readable$fromTransformer.transform(Readable.scala:13)
  upickle.Api.$anonfun$read$1(Api.scala:37)
  upickle.core.TraceVisitor$.withTrace(TraceVisitor.scala:21)
  upickle.Api.read(Api.scala:37)
  upickle.Api.read$(Api.scala:36)
  upickle.default$.read(Api.scala:157)
  ammonite.$sess.cmd4$.<clinit>(cmd4.sc:1)
aboisvert commented 3 months ago

@lihaoyi said:

What API are you using? It's meant to give you the character offset that the failure occurred even without trace. Trace is optional only if you want the pretty JSON path

I was reading the JSON data into an AST instead of reading directly from a file, e.g.,

    val data = os.read(fileName)
    val jsonAst = ujson.read(data)
    upickle.default.read[MyType](jsonAst)

Especially for the case of unmarshalling from an AST, it would be generally helpful to provide a json path where the error occurred, assuming trace is on. (This would complement the index position in case reading directly from a file/stream)

If this is desirable, I could see about creating a PR for this.