BuiltJsonSerializers throws an Error on deserialization exception but it should?

[danielgomezrico]

My current use case is that I'm using chopper to send HTTP requests and made it to use serialization with built_value, so all requests are serialized/deserialized with this.

On Every HTTP request, I catch errors to avoid unhandled cases. I found out that there may be cases where I defined a backend response in a way but for some reason it started sending another json or missed some params.

I want to monitor these cases and for that, I need to handle this case in my app.

Current

In the current implementation of the json serializers, it throws a DeserializationError and if you catch that in your code there's a linter that rise:

• Lint: https://dart-lang.github.io/linter/lints/avoid_catching_errors.html

The error is thrown here:

• built_value.dart/built_value/lib/src/built_json_serializers.dart

  Line 180 in 8b3dd1b

  throw DeserializationError(object, specifiedType, error);

Proposal

Return an Exception instead of an Error because this is something that you as a developer must handle, right? In your workflow, this kind of error may occur and having an error

[davidmorgan]

You probably should catch Error at the HTTP request level; Effective Dart says

https://dart.dev/effective-dart/usage#avoid-catches-without-on-clauses

"In rare cases, you may wish to catch any runtime error. This is usually in framework or low-level code that tries to insulate arbitrary application code from causing problems."

Isolating request handling so that a bad request does not break your server is exactly such a case. Requests are untrusted data and you do not want to create a path where unexpected/wrong data triggers a different codepath, that is allowing people to manipulate the server.

So while it's true that the built_value error would more correctly be an Exception, I think the calling code should catch anything anyway, so changing it does not seem like an improvement, in fact it might encourage people to do the wrong thing.

Please just ignore the lint in this case :)

[danielgomezrico]

Thanks for the answer David, what do you mean by this?

"it might encourage people to do the wrong thing."

[davidmorgan]

I meant that if I change it to an Exception, it might encourage code that only catches that exception, when usually the right thing at that point in the code is to catch anything. That way you catch any kind of range error, type error, etc that could be caused by a wrong request.

[danielgomezrico]

Got it, what you say makes sense 🤔