Make `Object.toString` `@mustBeOverriden` in Exception and Error

[gaaclarke]

The problem

I was using the package flame_tiled which parses xml files generated by the Tiled app and draws the results. I was getting an exception when parsing the file, but the log to console and to the screen only read "Instance of 'Parsing Exception'". With the stacktrace I was able to find the line of code generating the exception and see the exception class. I noticed that it is taking in data related to the failure, but since it never overrode Object.toString that information was never reported back to me.

By hacking the code in my ~/.pub-cache I was able to edit the code and debug the problem.

code:

part of tiled;

class ParsingException implements Exception {
  final String name;
  final String? valueFound;
  final String reason;
  ParsingException(this.name, this.valueFound, this.reason);
}

screenshot:

Proposal

I think it is an error to subclass Error or Exception and not override Object.toString. If someone wants the Object.toString they should explicitly have to opt into that, instead of getting it implicitly. We should have an annotation that forces subclasses to override toString for this case.

abstract interface class Exception {
  @mustBeOverriden
  String toString();
}

Alternatives considered

Here are some thoughts:

1. Migrate people from using Object.toString by introducing a new abstract method, like String what(). There is too much written code that relies on using Object.toString.
2. Make Object.toString abstract. This is obviously super breaking.
3. Leave it as is. This creates scenarios like the above that are unhelpful for users, good implementations of Exception and Error will override toString anyways. It will only be breaking poorly written subclasses.

[dart-github-bot]

Summary: flame_tiled's ParsingException lacked a toString() override, resulting in unhelpful error messages. The proposed solution is to add @mustBeOverriden to Exception's toString().

[lrhn]

Not immediately possible, since the platform libraries cannot import package:meta, so it can't use @mustBeOverridden.

Also, just to be a little contrary, you should never need toString of an Exception.
Exceptions are meant to be caught and handled. At that point, they don't need to be logged, they're handled.
Not handling an exception is an error. (But I guess that's where this comes in. If your error catching code gets an exception, that should be treated as an error ... not the exception, but the fact that it wasn't caught and handled. The only way to say that is to say which exception wasn't handled. But the content of the exception isn't relevant at that point, so the class name might be enough, and the stack trace along which it should have been handled.)

[gaaclarke]

Also, just to be a little contrary, you should never need toString of an Exception.

Wether I should be getting uncaught Exceptions is not relevant because there is no way to enforce someone will never get them. In my example in the description, a helpful Exception.toString would have meant I could have avoided debugging/editing 3rd party code. The conceptual argument doesn't match real world problems =)

Not immediately possible, since the platform libraries cannot import package:meta, so it can't use @mustBeOverridden.

We have some annotations in dart:core, deprecated and override. We'd have to promote it.

[Wdestroier]

The same problem can happen with any class. Sometimes you need to inspect its values and you don't have toString implemented. Adding toString in all classes would increase the app size a lot and make it much easier to be reverse engineered.

The Exception class has an optional message parameter. ParsingException could have extended Exception and formatted the message with : super('Found value $value for name $name, because $reason.').