Skip to content

Tunnel underlying NSErrors through C++ Status #2808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 12, 2019
Merged
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
8 changes: 8 additions & 0 deletions Firestore/Example/Firestore.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@
5492E0BF2021555100B64F25 /* FSTFieldValueTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0B82021555100B64F25 /* FSTFieldValueTests.mm */; };
5492E0C72021557E00B64F25 /* FSTSerializerBetaTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0C12021557E00B64F25 /* FSTSerializerBetaTests.mm */; };
5492E0C92021557E00B64F25 /* FSTRemoteEventTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0C32021557E00B64F25 /* FSTRemoteEventTests.mm */; };
5493A424225F9990006DE7BA /* status_apple_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5493A423225F9990006DE7BA /* status_apple_test.mm */; };
5493A425225F9990006DE7BA /* status_apple_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5493A423225F9990006DE7BA /* status_apple_test.mm */; };
5493A426225F9990006DE7BA /* status_apple_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5493A423225F9990006DE7BA /* status_apple_test.mm */; };
5495EB032040E90200EBA509 /* CodableGeoPointTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5495EB022040E90200EBA509 /* CodableGeoPointTests.swift */; };
54995F6F205B6E12004EFFA0 /* leveldb_key_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54995F6E205B6E12004EFFA0 /* leveldb_key_test.cc */; };
549CCA5020A36DBC00BCEB75 /* sorted_set_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 549CCA4C20A36DBB00BCEB75 /* sorted_set_test.cc */; };
Expand Down Expand Up @@ -831,6 +834,7 @@
5492E0B82021555100B64F25 /* FSTFieldValueTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTFieldValueTests.mm; sourceTree = "<group>"; };
5492E0C12021557E00B64F25 /* FSTSerializerBetaTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTSerializerBetaTests.mm; sourceTree = "<group>"; };
5492E0C32021557E00B64F25 /* FSTRemoteEventTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTRemoteEventTests.mm; sourceTree = "<group>"; };
5493A423225F9990006DE7BA /* status_apple_test.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = status_apple_test.mm; sourceTree = "<group>"; };
5495EB022040E90200EBA509 /* CodableGeoPointTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CodableGeoPointTests.swift; sourceTree = "<group>"; };
54995F6E205B6E12004EFFA0 /* leveldb_key_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = leveldb_key_test.cc; sourceTree = "<group>"; };
549CCA4C20A36DBB00BCEB75 /* sorted_set_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = sorted_set_test.cc; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1251,6 +1255,7 @@
AB380D03201BC6E400D97691 /* ordered_code_test.cc */,
403DBF6EFB541DFD01582AA3 /* path_test.cc */,
54740A531FC913E500713A1A /* secure_random_test.cc */,
5493A423225F9990006DE7BA /* status_apple_test.mm */,
54A0352C20A3B3D7003E0143 /* status_test.cc */,
54A0352B20A3B3D7003E0143 /* status_test_util.h */,
54A0352D20A3B3D7003E0143 /* statusor_test.cc */,
Expand Down Expand Up @@ -3051,6 +3056,7 @@
862B1AC9EDAB309BBF4FB18C /* sorted_map_test.cc in Sources */,
4A62B708A6532DD45414DA3A /* sorted_set_test.cc in Sources */,
C9F96C511F45851D38EC449C /* status.pb.cc in Sources */,
5493A425225F9990006DE7BA /* status_apple_test.mm in Sources */,
4DC660A62BC2B6369DA5C563 /* status_test.cc in Sources */,
74985DE2C7EF4150D7A455FD /* statusor_test.cc in Sources */,
C5C01A1FB216DA4BA8BF1A02 /* stream_test.mm in Sources */,
Expand Down Expand Up @@ -3218,6 +3224,7 @@
86E6FC2B7657C35B342E1436 /* sorted_map_test.cc in Sources */,
8413BD9958F6DD52C466D70F /* sorted_set_test.cc in Sources */,
0D2D25522A94AA8195907870 /* status.pb.cc in Sources */,
5493A426225F9990006DE7BA /* status_apple_test.mm in Sources */,
C0AD8DB5A84CAAEE36230899 /* status_test.cc in Sources */,
DC48407370E87F2233D7AB7E /* statusor_test.cc in Sources */,
215643858470A449D3A3E168 /* stream_test.mm in Sources */,
Expand Down Expand Up @@ -3460,6 +3467,7 @@
549CCA5220A36DBC00BCEB75 /* sorted_map_test.cc in Sources */,
549CCA5020A36DBC00BCEB75 /* sorted_set_test.cc in Sources */,
618BBEB120B89AAC00B5BCE7 /* status.pb.cc in Sources */,
5493A424225F9990006DE7BA /* status_apple_test.mm in Sources */,
54A0352F20A3B3D8003E0143 /* status_test.cc in Sources */,
54A0353020A3B3D8003E0143 /* statusor_test.cc in Sources */,
B66D8996213609EE0086DA0C /* stream_test.mm in Sources */,
Expand Down
2 changes: 0 additions & 2 deletions Firestore/Source/API/FIRFirestore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@

NS_ASSUME_NONNULL_BEGIN

extern "C" NSString *const FIRFirestoreErrorDomain = @"FIRFirestoreErrorDomain";

#pragma mark - FIRFirestore

@interface FIRFirestore ()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,10 @@
#import <FirebaseCore/FIRComponentContainer.h>
#import <FirebaseCore/FIROptionsInternal.h>

#include "Firestore/core/src/firebase/firestore/util/error_apple.h"
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"

// NB: This is also defined in Firestore/Source/Public/FIRFirestoreErrors.h
// NOLINTNEXTLINE: public constant
NSString* const FIRFirestoreErrorDomain = @"FIRFirestoreErrorDomain";

namespace firebase {
namespace firestore {
namespace auth {
Expand Down
2 changes: 2 additions & 0 deletions Firestore/core/src/firebase/firestore/util/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ cc_library(
comparison.h
config.h
delayed_constructor.h
error_apple.h
error_apple.mm
hashing.h
iterator_adaptors.h
objc_compatibility.h
Expand Down
28 changes: 20 additions & 8 deletions Firestore/core/src/firebase/firestore/util/error_apple.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,43 @@

#import <Foundation/Foundation.h>

#import <FirebaseFirestore/FIRFirestoreErrors.h> // for FIRFirestoreErrorDomain

#include "Firestore/core/include/firebase/firestore/firestore_errors.h"
#include "Firestore/core/src/firebase/firestore/util/status.h"
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
#include "absl/strings/string_view.h"

// The Cloud Firestore error domain. Keep in sync with FIRFirestoreErrors.h.
// Exposed here to make it possible to build in CMake without bringing in the
// sources under Firestore/Source.
FOUNDATION_EXPORT NSString* const FIRFirestoreErrorDomain
NS_SWIFT_NAME(FirestoreErrorDomain);

namespace firebase {
namespace firestore {
namespace util {

// Translates a set of error_code and error_msg to an NSError.
inline NSError* MakeNSError(const int64_t error_code,
const absl::string_view error_msg) {
const absl::string_view error_msg,
NSError* cause = nil) {
if (error_code == FirestoreErrorCode::Ok) {
return nil;
}
return [NSError
errorWithDomain:FIRFirestoreErrorDomain
code:static_cast<NSInteger>(error_code)
userInfo:@{NSLocalizedDescriptionKey : WrapNSString(error_msg)}];

NSMutableDictionary<NSString*, id>* user_info =
[NSMutableDictionary dictionary];
user_info[NSLocalizedDescriptionKey] = WrapNSString(error_msg);
if (cause) {
user_info[NSUnderlyingErrorKey] = cause;
}

return [NSError errorWithDomain:FIRFirestoreErrorDomain
code:static_cast<NSInteger>(error_code)
userInfo:user_info];
}

inline NSError* MakeNSError(const util::Status& status) {
return MakeNSError(status.code(), status.error_message());
return status.ToNSError();
}

inline Status MakeStatus(NSError* error) {
Expand Down
22 changes: 22 additions & 0 deletions Firestore/core/src/firebase/firestore/util/error_apple.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright 2019 Google
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "Firestore/core/src/firebase/firestore/util/error_apple.h"

// NB: This is also defined in Firestore/Source/Public/FIRFirestoreErrors.h
// NOLINTNEXTLINE: public constant
FOUNDATION_EXPORT NSString* const FIRFirestoreErrorDomain =
@"FIRFirestoreErrorDomain";
17 changes: 17 additions & 0 deletions Firestore/core/src/firebase/firestore/util/status.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "Firestore/core/src/firebase/firestore/util/status.h"

#include <ostream>
#include <utility>

#include "Firestore/core/src/firebase/firestore/util/string_format.h"
#include "absl/memory/memory.h"
Expand Down Expand Up @@ -49,6 +50,22 @@ Status& Status::CausedBy(const Status& cause) {
}

absl::StrAppend(&state_->msg, ": ", cause.error_message());

// If this Status has no accompanying PlatformError but the cause does, create
// an PlatformError for this Status ahead of time to preserve the causal chain
// that Status doesn't otherwise support.
if (state_->platform_error == nullptr &&
cause.state_->platform_error != nullptr) {
state_->platform_error =
cause.state_->platform_error->WrapWith(code(), error_message());
}

return *this;
}

Status& Status::WithPlatformError(std::unique_ptr<PlatformError> error) {
HARD_ASSERT(!ok(), "Platform errors should not be applied to Status::OK()");
state_->platform_error = std::move(error);
return *this;
}

Expand Down
39 changes: 38 additions & 1 deletion Firestore/core/src/firebase/firestore/util/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ namespace firebase {
namespace firestore {
namespace util {

class PlatformError;

/// Denotes success or failure of a call.
class ABSL_MUST_USE_RESULT Status {
public:
Expand Down Expand Up @@ -63,6 +65,8 @@ class ABSL_MUST_USE_RESULT Status {

#if defined(__OBJC__)
static Status FromNSError(NSError* error);

NSError* ToNSError() const;
#endif // defined(__OBJC__)

/// Returns true iff the status indicates success.
Expand Down Expand Up @@ -97,6 +101,8 @@ class ABSL_MUST_USE_RESULT Status {
/// \return *this
Status& CausedBy(const Status& cause);

Status& WithPlatformError(std::unique_ptr<PlatformError> error);

/// \brief Return a string representation of this status suitable for
/// printing. Returns the string `"OK"` for success.
std::string ToString() const;
Expand All @@ -110,20 +116,51 @@ class ABSL_MUST_USE_RESULT Status {
private:
static const std::string& empty_string();
struct State {
State() = default;
State(const State& other);

FirestoreErrorCode code;
std::string msg;

// An additional platform-specific error representation that was used to
// generate this Status. The PlatformError does not meaningfully contribute
// to the identity of this Status: it exists to allow tunneling e.g.
// NSError* to Status and back to NSError* losslessly.
std::unique_ptr<PlatformError> platform_error;
};
// OK status has a `NULL` state_. Otherwise, `state_` points to
// OK status has a `nullptr` state_. Otherwise, `state_` points to
// a `State` structure containing the error code and message(s)
std::unique_ptr<State> state_;

void SlowCopyFrom(const State* src);
};

class PlatformError {
public:
virtual ~PlatformError() {
}

virtual std::unique_ptr<PlatformError> Copy() = 0;

/**
* Creates a new PlatformError with the given code and message, whose cause is
* this PlatformError.
*/
virtual std::unique_ptr<PlatformError> WrapWith(FirestoreErrorCode code,
std::string message) = 0;
};

inline Status::Status(const Status& s)
: state_((s.state_ == nullptr) ? nullptr : new State(*s.state_)) {
}

inline Status::State::State(const State& s)
: code(s.code),
msg(s.msg),
platform_error((s.platform_error == nullptr) ? nullptr
: s.platform_error->Copy()) {
}

inline void Status::operator=(const Status& s) {
// The following condition catches both aliasing (when this == &s),
// and the common case where both s and *this are ok.
Expand Down
55 changes: 52 additions & 3 deletions Firestore/core/src/firebase/firestore/util/status_apple.mm
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,79 @@

#if defined(__APPLE__)

#include "Firestore/core/src/firebase/firestore/util/error_apple.h"
#include "Firestore/core/src/firebase/firestore/util/string_format.h"
#include "absl/memory/memory.h"

namespace firebase {
namespace firestore {
namespace util {

class UnderlyingNSError : public PlatformError {
public:
explicit UnderlyingNSError(NSError* error) : error_(error) {
}

static std::unique_ptr<UnderlyingNSError> Create(NSError* error) {
return absl::make_unique<UnderlyingNSError>(error);
}

static NSError* Recover(
const std::unique_ptr<PlatformError>& platform_error) {
if (platform_error == nullptr) {
return nil;
}

return static_cast<UnderlyingNSError*>(platform_error.get())->error();
}

std::unique_ptr<PlatformError> Copy() override {
return absl::make_unique<UnderlyingNSError>(error_);
}

std::unique_ptr<PlatformError> WrapWith(FirestoreErrorCode code,
std::string message) override {
NSError* chain = MakeNSError(code, message, error_);
return Create(chain);
}

NSError* error() const {
return error_;
}

private:
NSError* error_;
};

Status Status::FromNSError(NSError* error) {
if (!error) {
return Status::OK();
}

NSError* original = error;
auto original = UnderlyingNSError::Create(error);

while (error) {
if ([error.domain isEqualToString:NSPOSIXErrorDomain]) {
return FromErrno(static_cast<int>(error.code),
MakeString(original.localizedDescription));
MakeString(original->error().localizedDescription))
.WithPlatformError(std::move(original));
}

error = error.userInfo[NSUnderlyingErrorKey];
}

return Status{FirestoreErrorCode::Unknown,
StringFormat("Unknown error: %s", original)};
StringFormat("Unknown error: %s", original->error())}
.WithPlatformError(std::move(original));
}

NSError* Status::ToNSError() const {
if (ok()) return nil;

NSError* error = UnderlyingNSError::Recover(state_->platform_error);
if (error) return error;

return MakeNSError(code(), error_message());
}

} // namespace util
Expand Down
4 changes: 4 additions & 0 deletions Firestore/core/src/firebase/firestore/util/status_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ static FirestoreErrorCode CodeForErrno(int errno_code) {
}

Status Status::FromErrno(int errno_code, absl::string_view msg) {
if (errno_code == 0) {
return Status::OK();
}

FirestoreErrorCode canonical_code = CodeForErrno(errno_code);
return Status{canonical_code,
util::StringFormat("%s (errno %s: %s)", msg, errno_code,
Expand Down
1 change: 1 addition & 0 deletions Firestore/core/test/firebase/firestore/util/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ cc_test(
hashing_test.cc
iterator_adaptors_test.cc
ordered_code_test.cc
status_apple_test.mm
status_test.cc
status_test_util.h
statusor_test.cc
Expand Down
Loading