Minor error handling improvements (#8061)

* Throw more specific exceptions when the error is due to bad input (backport of cr/359824564) (note that this only applies to the Unity build);
* Always log exceptions to make sure they make it into the log (regardless of exception handling by the user and any potential issues with the exception propagation mechanism).

#no-changelog

Author: var-const

Firestore/Example/Tests/Integration/API/FIRValidationTests.mm

@@ -303,7 +303,7 @@ - (void)testBatchWritesWithIncorrectReferencesFail {
   FIRFirestore *db2 = [self firestore];
   XCTAssertNotEqual(db1, db2);
 
-  NSString *reason = @"Provided document reference is from a different Firestore instance.";
+  NSString *reason = @"Provided document reference is from a different Cloud Firestore instance.";
   id data = @{@"foo" : @1};
   FIRDocumentReference *badRef = [db2 documentWithPath:@"foo/bar"];
   FIRWriteBatch *batch = [db1 batch];
@@ -318,7 +318,7 @@ - (void)testTransactionWritesWithIncorrectReferencesFail {
   FIRFirestore *db2 = [self firestore];
   XCTAssertNotEqual(db1, db2);
 
-  NSString *reason = @"Provided document reference is from a different Firestore instance.";
+  NSString *reason = @"Provided document reference is from a different Cloud Firestore instance.";
   id data = @{@"foo" : @1};
   FIRDocumentReference *badRef = [db2 documentWithPath:@"foo/bar"];
 

Firestore/Source/API/FIRTransaction.mm

@@ -178,7 +178,8 @@ - (FIRDocumentSnapshot *_Nullable)getDocument:(FIRDocumentReference *)document
 
 - (void)validateReference:(FIRDocumentReference *)reference {
   if (reference.firestore != self.firestore) {
-    ThrowInvalidArgument("Provided document reference is from a different Firestore instance.");
+    ThrowInvalidArgument("Provided document reference is from a different Cloud Firestore "
+                         "instance.");
   }
 }
 

Firestore/core/src/api/document_reference.cc

@@ -34,6 +34,7 @@
 #include "Firestore/core/src/model/precondition.h"
 #include "Firestore/core/src/model/resource_path.h"
 #include "Firestore/core/src/util/error_apple.h"
+#include "Firestore/core/src/util/firestore_exceptions.h"
 #include "Firestore/core/src/util/hard_assert.h"
 #include "Firestore/core/src/util/hashing.h"
 #include "Firestore/core/src/util/status.h"
@@ -62,7 +63,7 @@ DocumentReference::DocumentReference(model::ResourcePath path,
                                      std::shared_ptr<Firestore> firestore)
     : firestore_{std::move(firestore)} {
   if (path.size() % 2 != 0) {
-    HARD_FAIL(
+    util::ThrowInvalidArgument(
         "Invalid document reference. Document references must have an even "
         "number of segments, but %s has %s",
         path.CanonicalString(), path.size());

Firestore/core/src/api/firestore.cc

@@ -104,11 +104,17 @@ const Settings& Firestore::settings() const {
 void Firestore::set_settings(const Settings& settings) {
   std::lock_guard<std::mutex> lock{mutex_};
   if (client_) {
-    HARD_FAIL(
+    util::ThrowIllegalState(
         "Firestore instance has already been started and its settings can "
         "no longer be changed. You can only set settings before calling any "
         "other methods on a Firestore instance.");
   }
+  if (!settings.ssl_enabled() && settings.host() == Settings::DefaultHost) {
+    util::ThrowIllegalState(
+        "You can't set the 'sslEnabled' setting unless you also set a "
+        "non-default 'host'.");
+  }
+
   settings_ = settings;
 }
 

Firestore/core/src/api/write_batch.cc

@@ -80,8 +80,8 @@ void WriteBatch::VerifyNotCommitted() const {
 void WriteBatch::ValidateReference(const DocumentReference& reference) const {
   if (reference.firestore() != firestore_) {
     ThrowInvalidArgument(
-        "Provided document reference is from a different "
-        "Firestore instance.");
+        "Provided document reference is from a different Cloud Firestore "
+        "instance.");
   }
 }
 

Firestore/core/src/model/resource_path.cc

@@ -20,7 +20,8 @@
 #include <utility>
 #include <vector>
 
-#include "Firestore/core/src/util/hard_assert.h"
+#include "Firestore/core/src/util/exception.h"
+#include "absl/strings/match.h"
 #include "absl/strings/str_join.h"
 #include "absl/strings/str_split.h"
 
@@ -37,8 +38,10 @@ ResourcePath ResourcePath::FromStringView(absl::string_view path) {
   // sequences (e.g. __id123__) and just passes them through raw (they exist
   // for legacy reasons and should not be used frequently).
 
-  HARD_ASSERT(path.find("//") == std::string::npos,
-              "Invalid path (%s). Paths must not contain // in them.", path);
+  if (absl::StrContains(path, "//")) {
+    util::ThrowInvalidArgument(
+        "Invalid path (%s). Paths must not contain // in them.", path);
+  }
 
   // SkipEmpty because we may still have an empty segment at the beginning or
   // end if they had a leading or trailing slash (which we allow).

Firestore/core/src/util/exception.cc

@@ -52,17 +52,23 @@ ABSL_ATTRIBUTE_NORETURN void DefaultThrowHandler(ExceptionType type,
   }
   absl::StrAppend(&what, message);
 
+  // Always log the error -- it helps if there are any issues with the exception
+  // propagation mechanism and also makes sure the exception makes it into the
+  // log regardless of how it's handled.
+  LOG_ERROR("%s", what);
+
 #if ABSL_HAVE_EXCEPTIONS
   switch (type) {
     case ExceptionType::AssertionFailure:
       throw FirestoreInternalError(what);
     case ExceptionType::IllegalState:
-      throw std::logic_error(what);
+      // Omit descriptive text since the type already encodes the kind of error.
+      throw std::logic_error(message);
     case ExceptionType::InvalidArgument:
-      throw std::invalid_argument(what);
+      // Omit descriptive text since the type already encodes the kind of error.
+      throw std::invalid_argument(message);
   }
 #else
-  LOG_ERROR("%s", what);
   std::terminate();
 #endif