Moshi: Accumulate error strategy

[stojan26]

The KotlinJsonAdapter fails fast when it encounters an error. This mean if there is a problem with the JSON we only get the first error. Of course the benefit of this is we don't parse additional data when we know the whole process is gonna fail.

In some cases it might be useful to accumulate the errors and fail at the end of the whole process. This means more information about everything that is wrong with the data.

Since this is a tradeoff I suggest a new adapter that accumulates errors or a setting in the existing one.

Would you interested in a PR that adds that functionality?

[swankjesse]

I don‘t think we can do this well without adding significant complexity or performance costs. We might be able to tell you locally about all of the fields in a single class, but even that is awkward.

[stojan26]

Good point. When I wrote this I was thinking about locally - in a single class. The way auto-value does it.

      String missing = "";
      if (this.name == null) {
        missing += " name";
      }
      if (this.numberOfLegs == null) {
        missing += " numberOfLegs";
      }
      if (!missing.isEmpty()) {
        throw new IllegalStateException("Missing required properties:" + missing);
      }

[swankjesse]

Hmm... that isn't so bad actually. Maybe for single classes the cost is low enough?

[stojan26]

It should not be that hard/costly for a single class. And it could be quite useful to have all errors in one go.

I'll try and do a prototype next week and we can go from there.

[swankjesse]

Cool! Kotlin reflect, then codegen?

[swankjesse]

Lets do it like this:

  fun fromJson(reader: JsonReader): GetterAndSetter {
    var a: Int? = null
    var b: Int? = null
    var total: Int? = null
    accumulateErrors {
      reader.beginObject()
      while (reader.hasNext()) {
        when (reader.selectName(options)) {
          0 -> a = intAdapter.fromJson(reader) ?: wasNull("a", reader)
          1 -> b = intAdapter.fromJson(reader) ?: wasNull("b", reader)
          2 -> total = intAdapter.fromJson(reader) ?: wasNull("total", reader)
          -1 -> reader.skipNameAndValue()
        }
      }
      reader.endObject()
      require("a", a)
      require("b", b)
    }
    var result = GetterAndSetter(
        a!!,
        b!!
    )
    result.total = total ?: result.total
    return result
  }

with this support:

inline fun accumulateErrors(block: ErrorsReporter.() -> Unit)

class ErrorsReporter {
  fun <T> wasNull(name: String, jsonReader: JsonReader): T?
  fun <T> require(name: String, value: T?)
}

We’ll need some Kotlin-specific runtime stuff for this.

[ZacSweers]

Iterated on this a bit - once we get stable inline classes and contracts, we can do a really clever thing with zero allocations (unless we do find errors, in which case we're bombing out anyway) and tricking the compiler to avoid unnecessary nullchecks

inline fun <T> accumulateErrors(block: ErrorsReporter.() -> T): T {
  return ErrorsReporter().block()
}

inline class ErrorsReporter(var errors: List<String> = emptyList()) {
  fun <T> wasNull(name: String, jsonReader: JsonReader): T? {
    errors += "$name was null at ${jsonReader.path}"
    return null
  }

  fun checkErrors() {
    check(errors.isEmpty()) { errors.joinToString("\n") }
  }
}

// Lie to the compiler here, knowing that we'll manually bomb out if any errors were recorded
public inline fun <T : Any> ErrorsReporter.require(name: String, value: T?) {
  contract {
    returns() implies (value != null)
  }
  if (value == null) {
    errors += "$name was absent"
  }
}

Then we can do this in code gen

override fun fromJson(reader: JsonReader): KCGResponse = accumulateErrors {
    var users: List<KCGUser>? = null
    var status: String? = null
    var isRealJson: Boolean? = null
    reader.beginObject()
    while (reader.hasNext()) {
      when (reader.selectName(options)) {
        0 -> users = listOfKCGUserAdapter.fromJson(reader) ?: wasNull("users", reader)
        1 -> status = stringAdapter.fromJson(reader) ?: wasNull("status", reader)
        2 -> isRealJson = booleanAdapter.fromJson(reader) ?: wasNull("isRealJson", reader)
        -1 -> {
          // Unknown name, skip it.
          reader.skipName()
          reader.skipValue()
        }
      }
    }
    reader.endObject()
    require("users", users)
    require("status", status)
    require("isRealJson", isRealJson)
    checkErrors()
    var result = KCGResponse(
        users = users,
        status = status,
        isRealJson = isRealJson)
    return@accumulateErrors result
  }
}

[JulioCottaN26]

Any news for this? It would avoid a lot of code being written by hand in my company just to check nullability of fields.

[ZacSweers]

No, the solution above can't be used until inline classes and contracts are stable