linkedin / linkedin/avro-util

NPEs thrown by serialization for mandatory fields could have better error messages

Open
#553 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
87
Forks
73
Avg merge
2h 50m
Merged PRs (30d)
1

Description

When serializing a record with mandatory fields, there is no null check. For a mandatory string field, fast-avro generates code like this:

```
if (((CharSequence) data.get(1)) instanceof Utf8) {
(encoder).writeString(((Utf8)((CharSequence) data.get(1))));
} else {
(encoder).writeString(((CharSequence) data.get(1)).toString());
}
```

Line 4 will throw a NPE due to calling `toString()` on the null field.

Throwing a NPE is not necessarily a problem in and of itself (and we likely need to preserve that behavior in order to be compatible with vanilla Avro) but a better error message would be welcome. For example, the generated code might instead look like:

```
Object value = data.get(1);
if (value instanceof Utf8) {
(encoder).writeString((Utf8) value);
} else if (value == null) {
throw new NullPointerException("Field fieldName is not allowed to be null");
} else {
(encoder).writeString(value.toString());
}
```

This adds one more check on the hot path, though in the end the VM must do this check anyway when dereferencing, so it might not matter in terms of bytecode/JIT.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

The issue names no file or test; start by searching the serializer or code-generation path for mandatory string fields and the shown writeString branch. Check the existing serialization tests, then verify that null mandatory values still fail with a clearer field-specific NullPointerException without changing non-null output or nullability behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.