Make associative container out-of-range exception provide missing key

Summary:
When the key is missing, the standard associative containers throw an exception like `unordered_map::at: key not found`. That's nice, but it would be even better if we would actually know what key is missing. This is not an issue when one is accessing the container directly, but when that access happens several levels deep, such as here within folly formatting logic, we have no way of knowing what key was missing. This poses some difficulties in presenting error to the user.
This change makes folly format throw a subclass of `std::out_of_range` exception on a missing key when a string keyed associative container is used for providing parameters. That subclass stores the actual key used so it can be accessed in the exception handler. Existing callers can still catch `std::out_of_range` so they should be unaffected by this change.

Reviewed By: ot, yfeldblum

Differential Revision: D6202184

fbshipit-source-id: b8a6740aaccc5d8914ad7d099c8b901427f00083

diff --git a/folly/Format-inl.h b/folly/Format-inl.h
index 60fdf2452c62a1a27768ed9668c0b588e8cf7525..b97375c1f8e6e9b4660fa531af74075872c8d78c 100644 (file)
--- a/folly/Format-inl.h
+++ b/folly/Format-inl.h
@@ -27,6 +27,7 @@
 
 #include <folly/Exception.h>
 #include <folly/FormatTraits.h>
+#include <folly/MapUtil.h>
 #include <folly/Traits.h>
 #include <folly/portability/Windows.h>
 
@@ -950,7 +951,10 @@ struct KeyableTraitsAssoc : public FormatTraitsBase {
   typedef typename T::key_type key_type;
   typedef typename T::value_type::second_type value_type;
   static const value_type& at(const T& map, StringPiece key) {
-    return map.at(KeyFromStringPiece<key_type>::convert(key));
+    if (auto ptr = get_ptr(map, KeyFromStringPiece<key_type>::convert(key))) {
+      return *ptr;
+    }
+    detail::throwFormatKeyNotFoundException(key);
   }
   static const value_type&
   at(const T& map, StringPiece key, const value_type& dflt) {

diff --git a/folly/Format.cpp b/folly/Format.cpp
index 380cddbe418788ecdfabad7e686c820997661fa0..9ad5395e8f04f507c4496c9ed046a26622680148 100644 (file)
--- a/folly/Format.cpp
+++ b/folly/Format.cpp
@@ -356,4 +356,15 @@ void insertThousandsGroupingUnsafe(char* start_buffer, char** end_buffer) {
 }
 } // namespace detail
 
+FormatKeyNotFoundException::FormatKeyNotFoundException(StringPiece key)
+    : std::out_of_range(kMessagePrefix.str() + key.str()) {}
+
+constexpr StringPiece const FormatKeyNotFoundException::kMessagePrefix;
+
+namespace detail {
+[[noreturn]] void throwFormatKeyNotFoundException(StringPiece key) {
+  throw FormatKeyNotFoundException(key);
+}
+} // namespace detail
+
 } // namespace folly

diff --git a/folly/Format.h b/folly/Format.h
index 3837d1079e9636b31d529a32d39f990e0805057d..b8df86fbfa1e8d02c54251011ea3b570df62e892 100644 (file)
--- a/folly/Format.h
+++ b/folly/Format.h
@@ -18,6 +18,7 @@
 #define FOLLY_FORMAT_H_
 
 #include <cstdio>
+#include <stdexcept>
 #include <tuple>
 #include <type_traits>
 
@@ -306,6 +307,33 @@ inline std::string svformat(StringPiece fmt, Container&& container) {
   return vformat(fmt, std::forward<Container>(container)).str();
 }
 
+/**
+ * Exception class thrown when a format key is not found in the given
+ * associative container keyed by strings. We inherit std::out_of_range for
+ * compatibility with callers that expect exception to be thrown directly
+ * by std::map or std::unordered_map.
+ *
+ * Having the key be at the end of the message string, we can access it by
+ * simply adding its offset to what(). Not storing separate std::string key
+ * makes the exception type small and noexcept-copyable like std::out_of_range,
+ * and therefore able to fit in-situ in exception_wrapper.
+ */
+class FormatKeyNotFoundException : public std::out_of_range {
+ public:
+  explicit FormatKeyNotFoundException(StringPiece key);
+
+  char const* key() const noexcept {
+    return what() + kMessagePrefix.size();
+  }
+
+ private:
+  static constexpr StringPiece const kMessagePrefix = "format key not found: ";
+};
+
+namespace detail {
+[[noreturn]] void throwFormatKeyNotFoundException(StringPiece key);
+} // namespace detail
+
 /**
  * Wrap a sequence or associative container so that out-of-range lookups
  * return a default value rather than throwing an exception.

diff --git a/folly/test/FormatTest.cpp b/folly/test/FormatTest.cpp
index 3bb739bb94f75b5a352eebea9122cf51fcc08e2a..e84bc86250c9b59b6b6e974fbf043a3d48bcef52 100644 (file)
--- a/folly/test/FormatTest.cpp
+++ b/folly/test/FormatTest.cpp
@@ -162,6 +162,12 @@ TEST(Format, Simple) {
   EXPECT_EQ("worldXX", svformat("{hello:X<7}", defaulted(m2, "meow")));
   EXPECT_EQ("meowXXX", sformat("{[none]:X<7}", defaulted(m2, "meow")));
   EXPECT_EQ("meowXXX", svformat("{none:X<7}", defaulted(m2, "meow")));
+  try {
+    svformat("{none:X<7}", m2);
+    EXPECT_FALSE(true) << "svformat should throw on missing key";
+  } catch (const FormatKeyNotFoundException& e) {
+    EXPECT_STREQ("none", e.key());
+  }
 
   // Test indexing in strings
   EXPECT_EQ("61 62", sformat("{0[0]:x} {0[1]:x}", "abcde"));