Skip to content

Commit

Permalink
Core/Startup: Warn when a config key isn't found in the config files
Browse files Browse the repository at this point in the history
Example: "Missing name Guild.SaveInterval in config file worldserver.conf, add "Guild.SaveInterval = 15" to this file"
(cherry picked from commit 06b3bca)

# Conflicts:
#	src/common/Configuration/Config.cpp
  • Loading branch information
DDuarte committed Mar 27, 2016
1 parent b2bd181 commit 6487e2f
Showing 1 changed file with 36 additions and 8 deletions.
44 changes: 36 additions & 8 deletions src/common/Configuration/Config.cpp
Expand Up @@ -21,6 +21,7 @@
#include <boost/property_tree/ptree.hpp>
#include <boost/property_tree/ini_parser.hpp>
#include "Config.h"
#include "Log.h"

using namespace boost::property_tree;

Expand Down Expand Up @@ -69,11 +70,18 @@ bool ConfigMgr::Reload(std::string& error)

std::string ConfigMgr::GetStringDefault(std::string const& name, const std::string& def) const
{
std::string value = _config.get<std::string>(ptree::path_type(name, '/'), def);

value.erase(std::remove(value.begin(), value.end(), '"'), value.end());

return value;
try
{
std::string value = _config.get<std::string>(ptree::path_type(name, '/'));
value.erase(std::remove(value.begin(), value.end(), '"'), value.end());
return value;
}
catch (boost::property_tree::ptree_bad_path)
{
TC_LOG_WARN("server.loading", "Missing name %s in config file %s, add \"%s = %s\" to this file",
name.c_str(), _filename.c_str(), name.c_str(), def.c_str());
return def;
}
}

bool ConfigMgr::GetBoolDefault(std::string const& name, bool def) const
Expand All @@ -84,20 +92,40 @@ bool ConfigMgr::GetBoolDefault(std::string const& name, bool def) const
val.erase(std::remove(val.begin(), val.end(), '"'), val.end());
return (val == "true" || val == "TRUE" || val == "yes" || val == "YES" || val == "1");
}
catch (std::exception const& /*ex*/)
catch (boost::property_tree::ptree_bad_path)
{
TC_LOG_WARN("server.loading", "Missing name %s in config file %s, add \"%s = %d\" to this file",
name.c_str(), _filename.c_str(), name.c_str(), def ? 1 : 0);
return def;
}
}

int ConfigMgr::GetIntDefault(std::string const& name, int def) const
{
return _config.get<int>(ptree::path_type(name, '/'), def);
try
{
return _config.get<int>(ptree::path_type(name, '/'));
}
catch (boost::property_tree::ptree_bad_path)
{
TC_LOG_WARN("server.loading", "Missing name %s in config file %s, add \"%s = %d\" to this file",
name.c_str(), _filename.c_str(), name.c_str(), def);
return def;
}
}

float ConfigMgr::GetFloatDefault(std::string const& name, float def) const
{
return _config.get<float>(ptree::path_type(name, '/'), def);
try
{
return _config.get<float>(ptree::path_type(name, '/'));
}
catch (boost::property_tree::ptree_bad_path)
{
TC_LOG_WARN("server.loading", "Missing name %s in config file %s, add \"%s = %f\" to this file",
name.c_str(), _filename.c_str(), name.c_str(), def);
return def;
}
}

std::string const& ConfigMgr::GetFilename()
Expand Down

5 comments on commit 6487e2f

@DDuarte
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that the key it's trying to find is missing from the cobf file? ptree_bad_data sounds like a malformatted value or similar (e.g wrong type). Which one is responsible for this specific crash? I'll check this later.

@DDuarte
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boost::property_tree::ptree_bad_data — Error indicating that translation from given value to the property tree data_type (or vice versa) failed. Derives from ptree_error.
boost::property_tree::ptree_bad_path — Error indicating that specified path does not exist. Derives from ptree_error.

A copy of your conf file can help. I think that it should crash/error on bad_data but not on bad_path.

@DDuarte
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question still stands, do we want it to crash or catch the exception and log the error?

@w1sht0l1v3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catch the exception and log imo...since afaik all conf variables have defaults hardcoded.

@DDuarte
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9431b94

Please sign in to comment.