-
Notifications
You must be signed in to change notification settings - Fork 79
=serialization #512 Finish ErrorEnvelope implementation #549
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,70 @@ 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<Failure: Error>(_ error: Failure) { | ||
| private let codableError: CodableError | ||
|
|
||
| public var error: Error { | ||
| self.codableError | ||
| } | ||
|
|
||
| enum CodingKeys: CodingKey { | ||
| case manifest | ||
| case error | ||
| } | ||
|
|
||
| public init<Failure: CodableError>(_ 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.codableError = error | ||
| } | ||
| } | ||
|
|
||
| // 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) | ||
| public init<Failure: Error>(_ 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.codableError = BestEffortStringError(representation: String(reflecting: Failure.self)) | ||
| } | ||
|
|
||
| enum CodingKeys: CodingKey { | ||
| case manifest | ||
| case error | ||
| // this is a cop out if we want to send back a message or just type name etc | ||
| public init(description: String) { | ||
| self.codableError = BestEffortStringError(representation: description) | ||
| } | ||
|
|
||
| public init(from decoder: Decoder) throws { | ||
| // guard let context = decoder.actorSerializationContext else { | ||
| // throw SerializationError.missingSerializationContext(ErrorEnvelope.self, details: "While decoding [\(ErrorEnvelope.self)], using [\(decoder)]") | ||
| // } | ||
| guard let context = decoder.actorSerializationContext else { | ||
| throw SerializationError.missingSerializationContext(decoder, ErrorEnvelope.self) | ||
| } | ||
|
|
||
| let container = try decoder.container(keyedBy: CodingKeys.self) | ||
|
|
||
| // 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) | ||
| 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.codableError = try codableErrorType.init(from: errorDecoder) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh interesting |
||
| } | ||
|
|
||
| // 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) | ||
| throw SerializationError.missingSerializationContext(encoder, ErrorEnvelope.self) | ||
| } | ||
|
|
||
| var container = encoder.container(keyedBy: CodingKeys.self) | ||
| try container.encode(context.serialization.outboundManifest(type(of: self.codableError as Any)), forKey: .manifest) | ||
|
|
||
| // 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) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't get this approach to work because
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, what you dod though is absolutely right 👍 |
||
| // } else { | ||
| try container.encode(context.outboundManifest(BestEffortStringError.self), forKey: .manifest) | ||
| try container.encode(BestEffortStringError(representation: "\(type(of: self.error as Any))"), forKey: .error) | ||
| // } | ||
| let errorEncoder = container.superEncoder(forKey: .error) | ||
| try self.codableError.encode(to: errorEncoder) | ||
| } | ||
| } | ||
|
|
||
| public struct BestEffortStringError: Error, Codable, CustomStringConvertible { | ||
| public struct BestEffortStringError: Error, Codable, Equatable, CustomStringConvertible { | ||
| let representation: String | ||
|
|
||
| public var description: String { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't seem like this change is required but adding them here to be consistent
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right yeah because we default to JSON so it works fine by using it. Thanks for adding here 👍 |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<Message: ActorMessage>( | ||
| public mutating func registerCodable<Message: Codable>( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to the PR. Even though
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right ok, thankfully I'm able to remove both those specialized functions soon :) |
||
| _ type: Message.Type, hint hintOverride: String? = nil, | ||
| serializer serializerOverride: SerializerID? = nil | ||
| ) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -165,6 +165,10 @@ public class Serialization { | |
| settings.registerManifest(CRDT.ORSet<Int>.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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
|
||
| self.settings = settings | ||
| self.metrics = system.metrics | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍