Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Context required for error messages when using nested Configurations #31

Open
hoylen opened this issue Feb 6, 2020 · 2 comments
Open

Comments

@hoylen
Copy link

hoylen commented Feb 6, 2020

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.

@joeconwaystk
Copy link
Contributor

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?

@hoylen
Copy link
Author

hoylen commented Feb 6, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants