From 12ef5bc9a48519e65302e85fab3df5afa8f2dcb7 Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 8 Apr 2020 02:27:49 -0700 Subject: [PATCH 1/2] =serialization #512 Finish ErrorEnvelope implementation Motivation: `ErrorEnvelope`'s handling of `Codable` errors not yet implemented. Modifications: Finish implementing `ErrorEnvelope`. Frequently ran into the error below and had to take a different approach. ``` value of protocol type 'Codable' (aka 'Decodable & Encodable') cannot conform to 'Encodable'; only struct/enum/class types can conform to protocols ``` Results: Resolves https://github.com/apple/swift-distributed-actors/issues/512 --- Sources/DistributedActors/ActorMessages.swift | 104 ++++++++++-------- .../Serialization+SerializerID.swift | 3 + .../Serialization+Settings.swift | 2 +- .../Serialization/Serialization.swift | 4 + .../SerializationTests.swift | 71 ++++++++++++ 5 files changed, 139 insertions(+), 45 deletions(-) diff --git a/Sources/DistributedActors/ActorMessages.swift b/Sources/DistributedActors/ActorMessages.swift index 0a23980c3..9723b0ebb 100644 --- a/Sources/DistributedActors/ActorMessages.swift +++ b/Sources/DistributedActors/ActorMessages.swift @@ -2,7 +2,7 @@ // // This source file is part of the Swift Distributed Actors open source project // -// Copyright (c) 2018-2019 Apple Inc. and the Swift Distributed Actors project authors +// Copyright (c) 2018-2020 Apple Inc. and the Swift Distributed Actors project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -80,72 +80,88 @@ extension Result: ActorMessage where Success: ActorMessage { // FIXME: only then } /// Generic transportable Error type, can be used to wrap error types and represent them as best as possible for transporting. -// FIXME: Needs better impl: https://github.com/apple/swift-distributed-actors/issues/512 public struct ErrorEnvelope: Error, ActorMessage { - public let error: Error + public typealias CodableError = Error & Codable - public init(_ error: Failure) { + private let _boxed: BoxedCodableError + + public var error: Error { + self._boxed.error + } + + public init(_ error: Failure) { if let alreadyAnEnvelope = error as? Self { self = alreadyAnEnvelope - } else if let codableError = error as? Error & Codable { - self.error = codableError } else { - // we we can at least carry the error type (not the whole string repr, since it may have information we'd rather not share though) - self.error = BestEffortStringError(representation: String(reflecting: Failure.self)) + self._boxed = .init(error) } } + public init(_ error: Failure) { + // we can at least carry the error type (not the whole string repr, since it may have information we'd rather not share though) + self._boxed = .init(BestEffortStringError(representation: String(reflecting: Failure.self))) + } + // this is a cop out if we want to send back a message or just type name etc public init(description: String) { - self.error = BestEffortStringError(representation: description) + self._boxed = .init(BestEffortStringError(representation: description)) } - enum CodingKeys: CodingKey { - case manifest - case error + public init(from decoder: Decoder) throws { + let container = try decoder.singleValueContainer() + self._boxed = try container.decode(BoxedCodableError.self) } - public init(from decoder: Decoder) throws { -// guard let context = decoder.actorSerializationContext else { -// throw SerializationError.missingSerializationContext(ErrorEnvelope.self, details: "While decoding [\(ErrorEnvelope.self)], using [\(decoder)]") -// } + public func encode(to encoder: Encoder) throws { + var container = encoder.singleValueContainer() + try container.encode(self._boxed) + } - let container = try decoder.container(keyedBy: CodingKeys.self) + private struct BoxedCodableError: Codable { + let error: CodableError - // FIXME: implement being able to encode and carry Codable errors -// // FIXME: serialization should offer to more easily perform manifest deserialization of a Codable inside another one -// let manifest = try container.decode(Serialization.Manifest.self, forKey: .manifest) -// -// if let ErrorType = try context.summonType(from: manifest) as? Codable.Type { -// ErrorType._decode(from: &bytes, using: JSONDecoder()) -// -// self.error = error -// } else { -// throw SerializationError.unableToDeserialize(hint: "Unable to summon Codable type for \(manifest)") -// } - self.error = try container.decode(BestEffortStringError.self, forKey: .error) - } + init(_ error: T) { + self.error = error + } - // FIXME: this likely fails in some cases - public func encode(to encoder: Encoder) throws { - guard let context: Serialization.Context = encoder.actorSerializationContext else { - throw SerializationError.missingSerializationContext(encoder, self) + enum CodingKeys: CodingKey { + case manifest + case error + } + + init(from decoder: Decoder) throws { + guard let context = decoder.actorSerializationContext else { + throw SerializationError.missingSerializationContext(decoder, BoxedCodableError.self) + } + + let container = try decoder.container(keyedBy: CodingKeys.self) + + let manifest = try container.decode(Serialization.Manifest.self, forKey: .manifest) + let errorType = try context.summonType(from: manifest) + + guard let codableErrorType = errorType as? CodableError.Type else { + throw SerializationError.unableToDeserialize(hint: "Error type \(errorType) is not Codable") + } + + let errorDecoder = try container.superDecoder(forKey: .error) + self.error = try codableErrorType.init(from: errorDecoder) } - var container = encoder.container(keyedBy: CodingKeys.self) + func encode(to encoder: Encoder) throws { + guard let context: Serialization.Context = encoder.actorSerializationContext else { + throw SerializationError.missingSerializationContext(encoder, BoxedCodableError.self) + } - // FIXME: implement being able to encode and carry Codable errors (!) -// if let codableError = self.error as? Codable { -// try container.encode(context.outboundManifest(type(of: self.error as Any)), forKey: .manifest) -// try container.encode(codableError, forKey: .error) -// } else { - try container.encode(context.outboundManifest(BestEffortStringError.self), forKey: .manifest) - try container.encode(BestEffortStringError(representation: "\(type(of: self.error as Any))"), forKey: .error) -// } + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(context.serialization.outboundManifest(type(of: self.error as Any)), forKey: .manifest) + + let errorEncoder = container.superEncoder(forKey: .error) + try self.error.encode(to: errorEncoder) + } } } -public struct BestEffortStringError: Error, Codable, CustomStringConvertible { +public struct BestEffortStringError: Error, Codable, Equatable, CustomStringConvertible { let representation: String public var description: String { diff --git a/Sources/DistributedActors/Serialization/Serialization+SerializerID.swift b/Sources/DistributedActors/Serialization/Serialization+SerializerID.swift index ed032de4f..b4c35aaeb 100644 --- a/Sources/DistributedActors/Serialization/Serialization+SerializerID.swift +++ b/Sources/DistributedActors/Serialization/Serialization+SerializerID.swift @@ -113,5 +113,8 @@ extension Serialization { // op log receptionist internal static let PushOps: SerializerID = .foundationJSON internal static let AckOps: SerializerID = .foundationJSON + + internal static let ErrorEnvelope: SerializerID = .foundationJSON + internal static let BestEffortStringError: SerializerID = .foundationJSON } } diff --git a/Sources/DistributedActors/Serialization/Serialization+Settings.swift b/Sources/DistributedActors/Serialization/Serialization+Settings.swift index 98d55d76f..07e355af1 100644 --- a/Sources/DistributedActors/Serialization/Serialization+Settings.swift +++ b/Sources/DistributedActors/Serialization/Serialization+Settings.swift @@ -149,7 +149,7 @@ extension Serialization.Settings { /// /// By doing this before system startup you can ensure a specific serializer is used for those messages. /// Make sure tha other nodes in the system are configured the same way though. - public mutating func registerCodable( + public mutating func registerCodable( _ type: Message.Type, hint hintOverride: String? = nil, serializer serializerOverride: SerializerID? = nil ) { diff --git a/Sources/DistributedActors/Serialization/Serialization.swift b/Sources/DistributedActors/Serialization/Serialization.swift index 97bfe78f3..97ae0ea1e 100644 --- a/Sources/DistributedActors/Serialization/Serialization.swift +++ b/Sources/DistributedActors/Serialization/Serialization.swift @@ -165,6 +165,10 @@ public class Serialization { settings.registerManifest(CRDT.ORSet.self, serializer: .protobufRepresentable) // settings.registerManifest(AnyDeltaCRDT.self, serializer: ReservedID.CRDTDeltaBox) // FIXME: so we cannot test the CRDT.Envelope+SerializationTests + // errors + settings.registerCodable(ErrorEnvelope.self) // TODO: can be removed once https://github.com/apple/swift/pull/30318 lands + settings.registerCodable(BestEffortStringError.self) // TODO: can be removed once https://github.com/apple/swift/pull/30318 lands + self.settings = settings self.metrics = system.metrics diff --git a/Tests/DistributedActorsTests/SerializationTests.swift b/Tests/DistributedActorsTests/SerializationTests.swift index 63d951b90..723cc2ab5 100644 --- a/Tests/DistributedActorsTests/SerializationTests.swift +++ b/Tests/DistributedActorsTests/SerializationTests.swift @@ -27,6 +27,7 @@ class SerializationTests: ActorSystemTestBase { settings.serialization.registerCodable(HasStringRef.self) settings.serialization.registerCodable(HasIntRef.self) settings.serialization.registerCodable(HasInterestingMessageRef.self) + settings.serialization.registerCodable(CodableTestingError.self) } } @@ -287,6 +288,66 @@ class SerializationTests: ActorSystemTestBase { } s2.shutdown().wait() } + + // ==== ------------------------------------------------------------------------------------------------------------ + // MARK: Error envelope serialization + + func test_serialize_errorEnvelope_stringDescription() throws { + let description = "BOOM!!!" + let errorEnvelope = ErrorEnvelope(description: description) + + var (manifest, bytes) = try shouldNotThrow { + try system.serialization.serialize(errorEnvelope) + } + + let back: ErrorEnvelope = try shouldNotThrow { + try system.serialization.deserialize(as: ErrorEnvelope.self, from: &bytes, using: manifest) + } + + guard let bestEffortStringError = back.error as? BestEffortStringError else { + throw self.testKit.error("\(back.error) is not BestEffortStringError") + } + + bestEffortStringError.representation.shouldEqual(description) + } + + func test_serialize_errorEnvelope_notCodableError() throws { + let notCodableError: NotCodableTestingError = .errorTwo + let errorEnvelope = ErrorEnvelope(notCodableError) + + var (manifest, bytes) = try shouldNotThrow { + try system.serialization.serialize(errorEnvelope) + } + + let back: ErrorEnvelope = try shouldNotThrow { + try system.serialization.deserialize(as: ErrorEnvelope.self, from: &bytes, using: manifest) + } + + guard let bestEffortStringError = back.error as? BestEffortStringError else { + throw self.testKit.error("\(back.error) is not BestEffortStringError") + } + + bestEffortStringError.representation.shouldContain(String(reflecting: NotCodableTestingError.self)) + } + + func test_serialize_errorEnvelope_codableError() throws { + let codableError: CodableTestingError = .errorB + let errorEnvelope = ErrorEnvelope(codableError) + + var (manifest, bytes) = try shouldNotThrow { + try system.serialization.serialize(errorEnvelope) + } + + let back: ErrorEnvelope = try shouldNotThrow { + try system.serialization.deserialize(as: ErrorEnvelope.self, from: &bytes, using: manifest) + } + + guard let codableTestingError = back.error as? CodableTestingError else { + throw self.testKit.error("\(back.error) is not CodableTestingError") + } + + codableTestingError.shouldEqual(codableError) + } } // MARK: Example types for serialization tests @@ -366,3 +427,13 @@ private struct NotSerializable { self.pos = pos } } + +private enum NotCodableTestingError: Error, Equatable { + case errorOne + case errorTwo +} + +private enum CodableTestingError: String, Error, Equatable, Codable { + case errorA + case errorB +} From 0a066cf2d02c8fcac40d4dcc87269ba2449371a9 Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 8 Apr 2020 16:27:27 -0700 Subject: [PATCH 2/2] Remove BoxedCodableError type --- Sources/DistributedActors/ActorMessages.swift | 74 +++++++------------ 1 file changed, 28 insertions(+), 46 deletions(-) diff --git a/Sources/DistributedActors/ActorMessages.swift b/Sources/DistributedActors/ActorMessages.swift index 9723b0ebb..f0a0df120 100644 --- a/Sources/DistributedActors/ActorMessages.swift +++ b/Sources/DistributedActors/ActorMessages.swift @@ -83,81 +83,63 @@ extension Result: ActorMessage where Success: ActorMessage { // FIXME: only then public struct ErrorEnvelope: Error, ActorMessage { public typealias CodableError = Error & Codable - private let _boxed: BoxedCodableError + private let codableError: CodableError public var error: Error { - self._boxed.error + self.codableError + } + + enum CodingKeys: CodingKey { + case manifest + case error } public init(_ error: Failure) { if let alreadyAnEnvelope = error as? Self { self = alreadyAnEnvelope } else { - self._boxed = .init(error) + self.codableError = error } } public init(_ error: Failure) { // we can at least carry the error type (not the whole string repr, since it may have information we'd rather not share though) - self._boxed = .init(BestEffortStringError(representation: String(reflecting: Failure.self))) + self.codableError = BestEffortStringError(representation: String(reflecting: Failure.self)) } // this is a cop out if we want to send back a message or just type name etc public init(description: String) { - self._boxed = .init(BestEffortStringError(representation: description)) + self.codableError = BestEffortStringError(representation: description) } public init(from decoder: Decoder) throws { - let container = try decoder.singleValueContainer() - self._boxed = try container.decode(BoxedCodableError.self) - } - - public func encode(to encoder: Encoder) throws { - var container = encoder.singleValueContainer() - try container.encode(self._boxed) - } - - private struct BoxedCodableError: Codable { - let error: CodableError - - init(_ error: T) { - self.error = error - } - - enum CodingKeys: CodingKey { - case manifest - case error + guard let context = decoder.actorSerializationContext else { + throw SerializationError.missingSerializationContext(decoder, ErrorEnvelope.self) } - init(from decoder: Decoder) throws { - guard let context = decoder.actorSerializationContext else { - throw SerializationError.missingSerializationContext(decoder, BoxedCodableError.self) - } + let container = try decoder.container(keyedBy: CodingKeys.self) - let container = try decoder.container(keyedBy: CodingKeys.self) + let manifest = try container.decode(Serialization.Manifest.self, forKey: .manifest) + let errorType = try context.summonType(from: manifest) - let manifest = try container.decode(Serialization.Manifest.self, forKey: .manifest) - let errorType = try context.summonType(from: manifest) + guard let codableErrorType = errorType as? CodableError.Type else { + throw SerializationError.unableToDeserialize(hint: "Error type \(errorType) is not Codable") + } - guard let codableErrorType = errorType as? CodableError.Type else { - throw SerializationError.unableToDeserialize(hint: "Error type \(errorType) is not Codable") - } + let errorDecoder = try container.superDecoder(forKey: .error) + self.codableError = try codableErrorType.init(from: errorDecoder) + } - let errorDecoder = try container.superDecoder(forKey: .error) - self.error = try codableErrorType.init(from: errorDecoder) + public func encode(to encoder: Encoder) throws { + guard let context: Serialization.Context = encoder.actorSerializationContext else { + throw SerializationError.missingSerializationContext(encoder, ErrorEnvelope.self) } - func encode(to encoder: Encoder) throws { - guard let context: Serialization.Context = encoder.actorSerializationContext else { - throw SerializationError.missingSerializationContext(encoder, BoxedCodableError.self) - } + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(context.serialization.outboundManifest(type(of: self.codableError as Any)), forKey: .manifest) - var container = encoder.container(keyedBy: CodingKeys.self) - try container.encode(context.serialization.outboundManifest(type(of: self.error as Any)), forKey: .manifest) - - let errorEncoder = container.superEncoder(forKey: .error) - try self.error.encode(to: errorEncoder) - } + let errorEncoder = container.superEncoder(forKey: .error) + try self.codableError.encode(to: errorEncoder) } }