safe_config icon indicating copy to clipboard operation
safe_config copied to clipboard

Context required for error messages when using nested Configurations

Open hoylen opened this issue 5 years ago • 2 comments

When an error occurs in a nested Configuration, there is no context of where it came from.

For example, consider the following program:

import 'dart:io';
import 'package:safe_config/safe_config.dart';

class AppConfig extends Configuration {
  AppConfig(String fileName) : super.fromFile(File(fileName));

  String name;
  String host;

  ServerConfig master;
  ServerConfig slave;
}

class ServerConfig extends Configuration {
  String host;
  int port;
}

void main(List<String> args) {
  try {
    final config = AppConfig(args[0]);
    print('success');
  } on ConfigurationException catch (e) {
    print(e);
  }
}

And the following invalid configuration file (it is missing the "host" for the "slave"):

name: "Missing host example"
host: local.example.com

master:
  host: master.example.com
  port: 443

slave:
  # host: slave.example.com
  port: 443

It prints out:

Invalid configuration data for 'AppConfig'. Missing required values: 'host'

But there is no indication of where exactly it was missing from. Is it the top level one, the one nested inside "master" or nested inside "slave"?

This makes finding and fixing errors in large/complex configuration files very difficult. Especially for users of the application, who don't have access to the source code and might not read any documentation about the expected configuration format.

It would be better if the exception said something like "Missing required value: slave:host". It would even be better if it could indicate the line number where the error occurred.

hoylen avatar Feb 06 '20 03:02 hoylen

Version 3.0.0 (currently in beta on pub; 3.0.0-2) provides the behavior you are looking for - can you give that a shot to see if it solves the issue?

itsjoeconway avatar Feb 06 '20 12:02 itsjoeconway

Good

This is good. The new version 3.0.0-b2 includes a list of key paths, which is a big improvement.

For the above example, the toString on the ConfigurationException produces:

Failed to read key 'slave' for 'AppConfig' -> missing required key(s): 'host'

And if the top-level "host" was missing, it produces this:

Failed to read 'AppConfig' -> missing required key(s): 'host'

So the information is in there. But now the error message is a bit long, going over two lines.

Better

It would be better if the message produced by toString was written for the end-user to read, rather than for the developer. Since "AppConfig" is only meaningful to someone who has access to the source code. Something like:

missing key: host

missing key: slave:host

Best

I don't know how much effort is involved, but it would even be better if ConfigurationException had a separate member for the key name(s), instead of embedding it into its message member as a single string that said 'missing required key(s): "host"'.

And what was thrown was a subclass of ConfigurationException (e.g. a new MissingKey), whose toString method would combine the key path (if any) and key name(s) to produce the string.

Note: I wouldn't call it MissingKeyConfigurationException, since the dartdoc generated documentation uses fixed width columns and that long class name probably won't fit.

That way, the program can generate its own error message if it wanted to.

P.S. Just noticed ConfigurationError and ConfigurationException do not implement or extend the Dart Error or Exception. Maybe they should? The dartdoc generated documentation puts exceptions in a separate section from other classes, which partially avoids the need to put "exception" or "error" in the name of the class.

hoylen avatar Feb 06 '20 22:02 hoylen