marshmallow-code / marshmallow-code/marshmallow

How to report errors for container fields?

Open
#879 11 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

backwards incompat feedback welcome refactor
Dominant language
Python
Stars
7.2k
Forks
738
Avg merge
1d 23h
Merged PRs (30d)
7

Description

Generally, a field name is associated a list of errors as strings. The exception to this is container fields. Those may return errors for the container and the contained field(s). In this case, rather than a list, a `dict` is used where the keys are the field names or the list index ([as string or as integer](https://github.com/marshmallow-code/marshmallow/issues/493)):

```python
errors = {
'_schema': [...],
'field': [...],
'nested': {
# What about errors in the Nested field itself?
'_schema': [...],
'field': [...],
},
'nested_many': {
# What about errors in the Nested field itself?
'0': {
'_schema': [...],
'field': [...],
},
},
'list': {
# What about errors in the List field itself?
'0': [...],
}
'list_nested': {
# What about errors in the List field itself?
'0': {
# What about errors in the Nested field itself?
'_schema': [...],
'field': [...],
},
},
'dict': {
# What about errors in the Dict field itself?
'0': {
'key': [...],
'value': [...],
}
}
'dict_nested': {
# What about errors in the Dict field itself?
'0': {
'key': [...],
'value': {
# What about errors in the Nested field itself?
'_schema': [...],
'field': [...],
},
}
}
}
```

This has been working relatively fine for a while. It leaves a hole as you can't return errors relative to the `Nested` field itself, for instance, while returning errors for the fields in the nested schema.

It is _generally_ not blocking, as the most frequent `Nested` field level error is the `missing` error, and when the field is missing, there is _generally_ no nested error, so we return

```python
nested: ['Missing data for required field.']
```

It is a shame that the structure (`dict` or `list`) depends on the error, but we could live with this.

Except we sometimes need to return errors for both the field and the nested schema, so we came up with this (https://github.com/marshmallow-code/marshmallow/pull/299):

```python
'nested': {
'_field': [...],
'_schema': [...],
'field': [...],
},
```

where `_field` stores errors about the field itself.

This is inconsistent, as I said in https://github.com/marshmallow-code/marshmallow/pull/299#issuecomment-401607834 and showed in https://github.com/marshmallow-code/marshmallow/pull/863 because the errors on the nested field itself may be reported in different places depending on whether there are errors in the nested schema. In other words, a `dict` with a `_field` attribute is used only if there are also errors in the nested schema, otherwise, the errors are returned as a strings, like for any field.

The same considerations apply to all container fields (`List`, `Dict`).

I can't think of an ideal solution to allow reporting errors from both the container field and its nested schema/fields.

We'd like to keep the use of `dict` for containers because it makes browsing the error structure convenient:

```python
assert 'Some error.' in errors['nested']['field']
```

This rules out putting nested errors under a dedicated `_nested` key.

On the other hand, putting errors on fields themselves in a dedicated `_field` key for all fields seems a bit overkill as well.

A trade-off would be to use the `_field` key for fields errors for all container fields and only for them, but do it consistently.

I suppose that was the intent of https://github.com/marshmallow-code/marshmallow/pull/299.

```python
errors = {
'_schema': [...],
'field': ['Missing data for required field.',...],
'nested': {
'_field': ['Missing data for required field.', 'Other error from schema level validator.'],
},
}
```

This introduces a distinction between containers and other fields.

It looks bad in the usual case where fields are missing, as shown above, because containers have their "missing data" message embedded in a `dict`.

Also, the implementation might be challenging: how do you know a field is a container when registering the error?

I'd like to keep it simple, but I'm out of ideas, so I'm appealing to collective wisdom.

I have the feeling that a good decision on this would avoid many corner case issues, so this should be set before piling-up patches.

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

Start by reviewing the error-shape examples and the referenced issue #493 and pull requests #299 and #863. The issue does not name files or tests, and work is not ready to begin until a consistent container-error representation and its acceptance criteria are decided.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.