Commit 3a2ccbf7 authored by Michal 'vorner' Vaner's avatar Michal 'vorner' Vaner
Browse files

[2377] Report error on !MANY_ERRORS as exception

In this case, the user should be aware of the error as much as possible.
In case of MANY_ERRORS, we can't do this, exception is able to handle
just one error.
parent cc62eeca
......@@ -45,11 +45,24 @@ public:
complete_(false)
{}
void reportError(const std::string& filename, size_t line,
const std::string& reason)
{
callbacks_.error(filename, line, reason);
if (!(options_ & MANY_ERRORS)) {
// In case we don't have the lenient mode, every error is fatal
// and we throw
ok_ = false;
complete_ = true;
isc_throw(MasterLoaderError, reason.c_str());
}
}
void pushSource(const std::string& filename) {
std::string error;
if (!lexer_.pushSource(filename.c_str(), &error)) {
reportError("", 0, error);
ok_ = false;
callbacks_.error("", 0, error);
}
initialized_ = true;
}
......@@ -125,36 +138,41 @@ public:
// Good, we loaded another one
++count;
} else if ((options_ & MANY_ERRORS) == 0) {
return (true);
ok_ = false;
complete_ = true;
// We don't have the exact error here, but it was reported
// by the error callback.
isc_throw(MasterLoaderError, "Invalid RR data");
}
} catch (const MasterLoaderError&) {
// This is a hack. We exclude the MasterLoaderError from the
// below case. Once we restrict the below to some smaller
// exception, we should remove this.
throw;
} catch (const isc::Exception& e) {
// TODO: Do we want to list expected exceptions here instead?
callbacks_.error(lexer_.getSourceName(),
lexer_.getSourceLine(),
e.what());
if ((options_ & MANY_ERRORS) != 0) {
// We want to continue. Try to read until the end of line
bool end = false;
do {
const MasterToken& token(lexer_.getNextToken());
switch (token.getType()) {
case MasterToken::END_OF_FILE:
// TODO: Try pop in case this is not the only
// source
return (true);
case MasterToken::END_OF_LINE:
end = true;
break;
default:
// Do nothing. This is just to make compiler
// happy
break;
}
} while (!end);
} else {
// We abort on first error. We are therefore done.
return (true);
}
// TODO: Once we do #2518, catch only the DNSTextError here,
// not isc::Exception. The rest should be just simply
// propagated.
reportError(lexer_.getSourceName(), lexer_.getSourceLine(),
e.what());
// We want to continue. Try to read until the end of line
bool end = false;
do {
const MasterToken& token(lexer_.getNextToken());
switch (token.getType()) {
case MasterToken::END_OF_FILE:
// TODO: Try pop in case this is not the only
// source
return (true);
case MasterToken::END_OF_LINE:
end = true;
break;
default:
// Do nothing. This is just to make compiler
// happy
break;
}
} while (!end);
}
}
// When there was a fatal error and ok is false, we say we are done.
......
......@@ -25,6 +25,15 @@ namespace dns {
class Name;
class RRClass;
/// \brief Error while loading by MasterLoader without specifying the
/// MANY_ERRORS option.
class MasterLoaderError : public isc::Exception {
public:
MasterLoaderError(const char* file, size_t line, const char* what) :
Exception(file, line, what)
{}
};
/// \brief A class able to load DNS master files
///
/// This class is able to produce a stream of RRs from a master file.
......@@ -100,6 +109,9 @@ public:
/// It returns true if the loading is done.
/// \throw isc::InvalidOperation when called after loading was done
/// already.
/// \throw MasterLoaderError when there's an error in the input master
/// file and the MANY_ERRORS is not specified. It never throws this
/// in case MANY_ERRORS is specified.
bool loadIncremental(size_t count_limit);
/// \brief Load everything
......@@ -107,6 +119,9 @@ public:
/// This simply calls loadIncremental until the loading is done.
/// \throw isc::InvalidOperation when called after loading was done
/// already.
/// \throw MasterLoaderError when there's an error in the input master
/// file and the MANY_ERRORS is not specified. It never throws this
/// in case MANY_ERRORS is specified.
void load() {
while (!loadIncremental(1000)) { // 1000 = arbitrary largish number
// Body intentionally left blank
......
......@@ -225,8 +225,8 @@ TEST_F(MasterLoaderTest, brokenZone) {
stringstream zone_stream(zone);
setLoader(zone_stream, Name("example.org."), RRClass::IN(),
MasterLoader::DEFAULT);
loader_->load();
EXPECT_EQ(1, errors_.size());
EXPECT_THROW(loader_->load(), MasterLoaderError);
EXPECT_EQ(1, errors_.size()) << errors_[0];
EXPECT_TRUE(warnings_.empty());
checkRR("example.org", RRType::SOA(), "ns1.example.org. "
......@@ -242,7 +242,7 @@ TEST_F(MasterLoaderTest, brokenZone) {
stringstream zone_stream(zone);
setLoader(zone_stream, Name("example.org."), RRClass::IN(),
MasterLoader::MANY_ERRORS);
loader_->load();
EXPECT_NO_THROW(loader_->load());
EXPECT_EQ(1, errors_.size());
EXPECT_TRUE(warnings_.empty());
checkRR("example.org", RRType::SOA(), "ns1.example.org. "
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment