Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions docs/content/result.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,55 @@ if (my_result) {
}
```

## Locating errors

`.what()` returns a message meant to be read by humans. When an error summarizes several failures, for example because more than one field is invalid, the message lists all of them:

```
Found 2 errors:
1) Failed to parse field 'inputFile': Size validation failed: Value expected to be greater than or equal to 5, but got 4.
2) Field named 'outputFile' not found.
```

If your program needs to know where each failure occurred, for instance to show each message next to the right input in a form, use `.details()` instead. It returns a `std::vector<rfl::ErrorDetail>` with one entry per failure:

```cpp
struct ErrorDetail {
using PathSegment = std::variant<std::string, size_t>;

// Field names and array indices, starting from the root.
std::vector<PathSegment> path;

// The message, without the "Failed to parse field '...': " prefixes.
std::string message;
};
```

Field names are spelled as they appear in the input, that is, after processors such as `rfl::SnakeCaseToCamelCase` or `rfl::SnakeCaseToKebabCase` (which `rfl::cli::read` uses) have been applied. This means the paths line up with the JSON schema generated by `rfl::json::to_schema`.

```cpp
struct FileSpec {
rfl::Validator<std::string, rfl::Size<rfl::Minimum<5>>> input_file;
std::string output_file;
};

struct Request {
std::vector<FileSpec> files;
};

const auto result = rfl::json::read<Request, rfl::SnakeCaseToCamelCase>(
R"({"files":[{"inputFile":"a.txt","outputFile":"a.out"},{"inputFile":"b"}]})");

for (const auto& detail : result.error().details()) {
// First iteration: path = {"files", 1, "inputFile"},
// message = "Size validation failed: ..."
// Second iteration: path = {"files", 1, "outputFile"},
// message = "Field named 'outputFile' not found."
}
```

An error that does not belong to any particular field, such as a syntax error in the document, is reported as a single detail with an empty path. Untagged variants are also reported as a single detail at the location of the variant, because reflect-cpp cannot know which alternative the input was meant to be. For `rfl::TaggedUnion`, the tag determines the alternative, so the paths extend into the fields of that alternative.

## `rfl::error`

`rfl::error("error message")` is a simple utility function for `rfl::Unexpected<Error>(Error("error message"))`.
Expand Down
1 change: 1 addition & 0 deletions include/rfl/Result.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace rfl {

using Error = rfl::internal::Error;
using ErrorDetail = rfl::internal::ErrorDetail;

/// To be returned when there is nothing to return, but there might be an error.
struct Nothing {};
Expand Down
85 changes: 83 additions & 2 deletions include/rfl/internal/Error.hpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,36 @@
#ifndef RFL_INTERNAL_ERROR_HPP_
#define RFL_INTERNAL_ERROR_HPP_

#include <cstddef>
#include <memory>
#include <string>
#include <utility>
#include <variant>
#include <vector>

#include "cold.hpp"

namespace rfl::internal {

/// Describes a single failure contained in an Error: where it occurred and
/// what went wrong.
struct ErrorDetail {
/// A field name or an array index.
using PathSegment = std::variant<std::string, size_t>;

/// The location of the failure, starting from the root of the value being
/// read. Field names are spelled as they appear in the input, that is, after
/// processors such as rfl::SnakeCaseToCamelCase have been applied. An empty
/// path refers to the root value itself.
std::vector<PathSegment> path;

/// The message describing the failure, without the "Failed to parse field
/// '...': " prefixes that Error::what() contains for every level of nesting.
std::string message;

bool operator==(const ErrorDetail&) const = default;
};

/// Defines the error class to be returned when something went wrong.
/// This is used throughout reflect-cpp for error handling.
class Error {
Expand All @@ -20,12 +46,22 @@ class Error {
/// @param _what The error message describing what went wrong (will be moved)
Error(std::string&& _what) : what_(std::move(_what)) {}

/// Constructs an error with a message and the individual failures it
/// summarizes.
/// @param _what The error message describing what went wrong
/// @param _details The individual failures, see details()
Error(std::string _what, std::vector<ErrorDetail> _details)
: what_(std::move(_what)),
details_(_details.empty()
? nullptr
: new std::vector<ErrorDetail>(std::move(_details))) {}

/// Destructor.
~Error() = default;

/// Copy constructor.
/// @param e The error to copy from
Error(const Error& e) = default;
Error(const Error& e) : what_(e.what_), details_(copy_details(e.details_)) {}

/// Move constructor.
/// @param e The error to move from
Expand All @@ -34,7 +70,13 @@ class Error {
/// Copy assignment operator.
/// @param _other The error to copy from
/// @return Reference to this error
Error& operator=(const Error& _other) = default;
Error& operator=(const Error& _other) {
if (this != &_other) {
what_ = _other.what_;
details_ = copy_details(_other.details_);
}
return *this;
}

/// Move assignment operator.
/// @param _other The error to move from
Expand All @@ -47,9 +89,48 @@ class Error {
/// from state
std::string what() && { return std::move(what_); }

/// Returns the individual failures summarized by what(), each with the
/// location at which it occurred. An error that was constructed from a
/// message only is reported as a single failure at the root.
std::vector<ErrorDetail> details() const& {
if (!details_) {
return {ErrorDetail{.path = {}, .message = what_}};
}
return *details_;
}

/// Moves the individual failures out of the Error object and leaves it in a
/// moved from state.
std::vector<ErrorDetail> details() && {
if (!details_) {
return {ErrorDetail{.path = {}, .message = std::move(what_)}};
}
return std::move(*details_);
}

private:
/// Errors are created and destroyed in many places, mostly as part of
/// rfl::Result. Deleting and copying the details out of line keeps that code
/// small, which matters for the performance of successful reads.
struct DeleteDetails {
RFL_COLD void operator()(std::vector<ErrorDetail>* _ptr) const {
delete _ptr;
}
};

using DetailsPtr = std::unique_ptr<std::vector<ErrorDetail>, DeleteDetails>;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@clockblocker Could you use rfl::Box here? Clearly, the implicit assumption here is that DetailsPtr should not be NULL, so rfl::Box establishes that more clearly.

Moreover, if you use rfl::Box, you won't have to implement the copy operations by hand.


RFL_COLD static DetailsPtr copy_details(const DetailsPtr& _ptr) {
return _ptr ? DetailsPtr(new std::vector<ErrorDetail>(*_ptr)) : nullptr;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is what I mean...rfl::Box can handle that more elegantly.

}

private:
/// Documents what went wrong
std::string what_;

/// The individual failures summarized by what_. Null, unless what_ was
/// assembled from failures at nested locations.
DetailsPtr details_;
};

} // namespace rfl::internal
Expand Down
15 changes: 15 additions & 0 deletions include/rfl/internal/cold.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#ifndef RFL_INTERNAL_COLD_HPP_
#define RFL_INTERNAL_COLD_HPP_

/// Marks a function that is rarely called, such as one that only runs when
/// something went wrong. Such functions are kept out of the code of their
/// callers, so that they do not slow down the common case.
#if defined(_MSC_VER)
#define RFL_COLD __declspec(noinline)
#elif defined(__GNUC__) || defined(__clang__)
#define RFL_COLD __attribute__((noinline, cold))
#else
#define RFL_COLD
#endif

#endif
7 changes: 5 additions & 2 deletions include/rfl/parsing/ArrayReader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@

#include <array>
#include <optional>
#include <string>
#include <type_traits>

#include "../Result.hpp"
#include "../internal/is_array.hpp"
#include "Parser_base.hpp"
#include "wrap_error.hpp"

namespace rfl::parsing {

Expand Down Expand Up @@ -62,8 +64,9 @@ class ArrayReader {
if (res) {
move_to(&((*array_)[num_set_]), &(*res));
} else {
return Error("Failed to parse element " + std::to_string(num_set_) +
": " + res.error().what());
return wrap_error(
"Failed to parse element " + std::to_string(num_set_) + ": ",
num_set_, std::move(res).error());
}
++num_set_;
return std::optional<Error>();
Expand Down
10 changes: 4 additions & 6 deletions include/rfl/parsing/FieldErrors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

#include <cstddef>
#include <optional>
#include <string>
#include <type_traits>
#include <utility>
#include <vector>
Expand All @@ -19,8 +18,8 @@ class FirstFieldError {

void reserve(std::size_t) {}

void emplace_back(std::string message) {
if (!error_) error_.emplace(std::move(message));
void emplace_back(Error _err) {
if (!error_) error_.emplace(std::move(_err));
}

std::optional<Error> error() && { return std::move(error_); }
Expand All @@ -30,9 +29,8 @@ class FirstFieldError {
};

template <class ProcessorsType>
using FieldErrors =
std::conditional_t<internal::fail_fast_v<ProcessorsType>, FirstFieldError,
std::vector<std::string>>;
using FieldErrors = std::conditional_t<internal::fail_fast_v<ProcessorsType>,
FirstFieldError, std::vector<Error>>;

template <class ProcessorsType>
bool should_stop_on_field_error(const FieldErrors<ProcessorsType>& errors) {
Expand Down
4 changes: 2 additions & 2 deletions include/rfl/parsing/MapParser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ struct MapParser {
}
return map;
} else {
std::vector<std::string> errors;
std::vector<Error> errors;
const auto map_reader =
MapReader<R, W, MapType, ProcessorsType>(&_r, &map, &errors);
if constexpr (schemaful::IsSchemafulReader<R>) {
Expand All @@ -111,7 +111,7 @@ struct MapParser {
}
}
if (errors.size() != 0) {
return error(to_single_error_message(errors));
return error(to_single_error(std::move(errors)));
}
return map;
}
Expand Down
8 changes: 4 additions & 4 deletions include/rfl/parsing/MapReader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "../always_false.hpp"
#include "../internal/has_reflection_type_v.hpp"
#include "Parser_base.hpp"
#include "wrap_error.hpp"

namespace rfl::parsing {

Expand All @@ -30,7 +31,7 @@ class MapReader {
* @param _map The map to write to.
* @param _errors The vector to collect errors in.
*/
MapReader(const R* _r, MapType* _map, std::vector<std::string>* _errors)
MapReader(const R* _r, MapType* _map, std::vector<Error>* _errors)
: r_(_r), map_(_map), errors_(_errors) {}

~MapReader() = default;
Expand All @@ -47,8 +48,7 @@ class MapReader {
if (res) {
map_->emplace(std::move(*res));
} else {
errors_->push_back("Failed to parse field '" + std::string(_name) +
"': " + res.error().what());
errors_->emplace_back(wrap_field_error(_name, std::move(res).error()));
}
}

Expand Down Expand Up @@ -122,7 +122,7 @@ class MapReader {
MapType* map_;

/// Collects any errors we may have come across.
std::vector<std::string>* errors_;
std::vector<Error>* errors_;
};

} // namespace rfl::parsing
Expand Down
8 changes: 5 additions & 3 deletions include/rfl/parsing/NamedTupleParser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "is_required.hpp"
#include "schema/Type.hpp"
#include "to_single_error_message.hpp"
#include "wrap_error.hpp"

namespace rfl {

Expand Down Expand Up @@ -348,7 +349,8 @@ struct NamedTupleParser {
std::stringstream stream;
stream << "Field named '" << std::string(current_name)
<< "' not found.";
_errors->emplace_back(stream.str());
_errors->emplace_back(
make_nested_error(std::string(current_name), stream.str()));

} else if constexpr (!internal::has_default_val_v<NamedTupleType>) {
if constexpr (!std::is_const_v<ValueType>) {
Expand Down Expand Up @@ -410,7 +412,7 @@ struct NamedTupleParser {
return std::make_pair(set, std::move(errors).error());
} else {
if (errors.size() != 0) [[unlikely]] {
return std::make_pair(set, to_single_error_message(errors));
return std::make_pair(set, to_single_error(std::move(errors)));
}
return std::make_pair(set, std::optional<Error>());
}
Expand Down Expand Up @@ -443,7 +445,7 @@ struct NamedTupleParser {
return std::move(errors).error();
} else {
if (errors.size() != 0) [[unlikely]] {
return to_single_error_message(errors);
return to_single_error(std::move(errors));
}
return std::nullopt;
}
Expand Down
8 changes: 4 additions & 4 deletions include/rfl/parsing/ParserTaggedUnion.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "schemaful/IsSchemafulReader.hpp"
#include "schemaful/IsSchemafulWriter.hpp"
#include "tagged_union_wrapper_no_ptr.hpp"
#include "wrap_error.hpp"

namespace rfl::parsing {

Expand Down Expand Up @@ -182,13 +183,12 @@ struct ParserTaggedUnion<R, W, TaggedUnion<_discriminator, AlternativeTypes...>,
std::move(_val));
};

const auto embellish_error = [&](auto&& _e) {
const auto embellish_error = [&](Error _e) {
std::stringstream stream;
stream << "Could not parse tagged union with "
"discriminator "
<< _discriminator.str() << " '" << _disc_value
<< "': " << _e.what();
return Error(stream.str());
<< _discriminator.str() << " '" << _disc_value << "': ";
return wrap_error(stream.str(), std::move(_e));
};

using T = tagged_union_wrapper_no_ptr_t<std::invoke_result_t<
Expand Down
2 changes: 1 addition & 1 deletion include/rfl/parsing/Parser_default.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ struct Parser {
Parser<R, W, ViewType, ProcessorsType>::read_view(_r, _var, &view);
if (err) [[unlikely]] {
call_destructors_where_necessary(set, &view);
return error(err->what());
return error(*err);
}
auto res = Result<T>(std::move(*ptr));
call_destructors_where_necessary(set, &view);
Expand Down
Loading
Loading