Skip to content

Commit

Permalink
[Enhancement]Improve Error logging
Browse files Browse the repository at this point in the history
  • Loading branch information
ipavlidakis committed Jan 30, 2025
1 parent cafeaef commit 3d73e42
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 44 deletions.
4 changes: 2 additions & 2 deletions DemoApp/Sources/Components/AppState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,15 @@ final class AppState: ObservableObject {
try await streamVideo?.deleteDevice(id: voipPushToken)
log.debug("✅ Removed VOIP push notification device for token: \(voipPushToken)")
} catch {
log.error("Removing VOIP push notification device for token: \(voipPushToken)")
log.error("Removing VOIP push notification device for token: \(voipPushToken)", error: error)
}
}
if let pushToken = unsecureRepository.currentPushToken() {
do {
try await streamVideo?.deleteDevice(id: pushToken)
log.debug("✅ Removed push notification device for token: \(pushToken)")
} catch {
log.error("Removing push notification device for token: \(pushToken)")
log.error("Removing push notification device for token: \(pushToken)", error: error)
}
}
await streamVideo?.disconnect()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ final class OSLogDestination: BaseLogDestination {

switch logDetails.level {
case .debug:
logger.debug("\(formattedMessage, privacy: .public)")
logger.debug("\(formattedMessage)")
case .info:
logger.notice("\(formattedMessage, privacy: .public)")
logger.notice("\(formattedMessage)")
case .warning:
logger.warning("\(formattedMessage, privacy: .public)")
logger.warning("\(formattedMessage)")
case .error:
logger.critical("\(formattedMessage, privacy: .public)")
logger.critical("\(formattedMessage)")
}
}
}
12 changes: 0 additions & 12 deletions DemoApp/Sources/Extensions/DemoApp+Sentry.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,6 @@ import Foundation
import Sentry
import StreamVideo

private struct DemoLogFormatter: LogFormatter {
func format(logDetails: LogDetails, message: String) -> String {
guard logDetails.level == .error, let error = logDetails.error else {
return message
}
return "\(message) [Error details: \(error)]"
}
}

func configureSentry() {
if AppEnvironment.configuration.isRelease {
// We're tracking Crash Reports / Issues from the Demo App to keep improving the SDK
Expand All @@ -40,9 +31,6 @@ func configureSentry() {
MemoryLogDestination.self,
OSLogDestination.self
]
LogConfig.formatters = [
DemoLogFormatter()
]
}
}

Expand Down
20 changes: 2 additions & 18 deletions Sources/StreamVideo/Errors/Errors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,10 @@ import Foundation

extension APIError: Error {}

extension Stream_Video_Sfu_Models_Error: Error, CustomStringConvertible {
var description: String {
if code == .unspecified {
return "-"
} else {
return """
code: \(code)
message: \(message)
shouldRetry: \(shouldRetry)
"""
}
}
}
extension Stream_Video_Sfu_Models_Error: Error, ReflectiveStringConvertible {}

/// A Client error.
public class ClientError: Error, CustomStringConvertible {
public class ClientError: Error, ReflectiveStringConvertible {
public struct Location: Equatable {
public let file: String
public let line: Int
Expand All @@ -47,10 +35,6 @@ public class ClientError: Error, CustomStringConvertible {
/// Retrieve the localized description for this error.
public var localizedDescription: String { message ?? errorDescription ?? "" }

public private(set) lazy var description = "Error \(type(of: self)) in \(location?.file ?? ""):\(location?.line ?? 0)"
+ (localizedDescription.isEmpty ? "" : " -> ")
+ localizedDescription

/// A client error based on an external general error.
/// - Parameters:
/// - error: an external error.
Expand Down
11 changes: 9 additions & 2 deletions Sources/StreamVideo/OpenApi/generated/Models/APIError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import Foundation

public final class APIError: @unchecked Sendable, Codable, JSONEncodable, Hashable {
public final class APIError: @unchecked Sendable, Codable, JSONEncodable, Hashable, ReflectiveStringConvertible {

public var code: Int
public var details: [Int]
public var duration: String
Expand All @@ -15,6 +15,13 @@ public final class APIError: @unchecked Sendable, Codable, JSONEncodable, Hashab
public var statusCode: Int
public var unrecoverable: Bool?

public var skipRuleSet: Set<ReflectiveStringConvertibleSkipRule> {
[
.empty,
.nilValues
]
}

public init(
code: Int,
details: [Int],
Expand Down
15 changes: 12 additions & 3 deletions Sources/StreamVideo/StreamVideo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,10 @@ public class StreamVideo: ObservableObject, @unchecked Sendable {
/// - Parameter id: the id of the device (token) for push notifications.
@discardableResult
public func setDevice(id: String) async throws -> ModelResponse {
try await setDevice(
guard !id.isEmpty else {
throw ClientError("Device id must not be empty when trying to set device.")
}
return try await setDevice(
id: id,
pushProvider: pushNotificationsConfig.pushProviderInfo.pushProvider,
name: pushNotificationsConfig.pushProviderInfo.name,
Expand All @@ -264,7 +267,10 @@ public class StreamVideo: ObservableObject, @unchecked Sendable {
/// - Parameter id: the id of the device (token) for VoIP push notifications.
@discardableResult
public func setVoipDevice(id: String) async throws -> ModelResponse {
try await setDevice(
guard !id.isEmpty else {
throw ClientError("Device id must not be empty when trying to set VoIP device.")
}
return try await setDevice(
id: id,
pushProvider: pushNotificationsConfig.voipPushProviderInfo.pushProvider,
name: pushNotificationsConfig.voipPushProviderInfo.name,
Expand All @@ -276,7 +282,10 @@ public class StreamVideo: ObservableObject, @unchecked Sendable {
/// - Parameter id: the id of the device that will be deleted.
@discardableResult
public func deleteDevice(id: String) async throws -> ModelResponse {
try await coordinatorClient.deleteDevice(id: id)
guard !id.isEmpty else {
throw ClientError("Device id must not be empty when trying to delete device.")
}
return try await coordinatorClient.deleteDevice(id: id)
}

/// Lists the devices registered for the user.
Expand Down
12 changes: 12 additions & 0 deletions Sources/StreamVideo/Utils/Logger/Logger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,18 @@ public class Logger {
fileName: StaticString = #fileID,
lineNumber: UInt = #line
) {
let error = {
if error is ClientError {
return error
} else if error is APIError {
return error
} else if let error {
return ClientError(with: error, fileName, lineNumber)
} else {
return nil
}
}()

log(
.error,
functionName: functionName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,104 @@

import Foundation

/// An enumeration representing rules for skipping properties during reflective
/// string conversion.
///
/// These rules can be used to exclude properties based on specific conditions,
/// such as being empty, nil, or matching a custom rule.
public enum ReflectiveStringConvertibleSkipRule: Hashable {
/// Skip properties that are empty.
case empty

/// Skip properties that are nil.
case nilValues

/// Skip properties based on a custom rule.
///
/// - Parameters:
/// - identifier: A unique identifier for the custom rule.
/// - rule: A closure that takes a `Mirror.Child` and returns a Boolean
/// indicating whether the property should be skipped.
case custom(identifier: String, rule: (Mirror.Child) -> Bool)

/// Hashes the essential components of this value by feeding them into the
/// given hasher.
///
/// - Parameter hasher: The hasher to use when combining the components of
/// this instance.
public func hash(into hasher: inout Hasher) {
switch self {
case .empty:
hasher.combine(".empty")
case .nilValues:
hasher.combine(".nilValues")
case let .custom(identifier, _):
hasher.combine(".custom_")
hasher.combine(identifier)
}
}

/// Determines whether a given property should be skipped based on the rule.
///
/// - Parameter child: A `Mirror.Child` representing the property to check.
/// - Returns: A Boolean indicating whether the property should be skipped.
public func shouldBeSkipped(_ child: Mirror.Child) -> Bool {
switch self {
case .empty:
if (child.value as? String)?.isEmpty == true {
return true
} else if (child.value as? (any Collection))?.isEmpty == true {
return true
} else {
return false
}

case .nilValues:
return "\(child.value)" == "nil"

case let .custom(_, rule):
return rule(child)
}
}

/// Compares two `ReflectiveStringConvertibleSkipRule` values for equality.
///
/// - Parameters:
/// - lhs: A `ReflectiveStringConvertibleSkipRule` value.
/// - rhs: Another `ReflectiveStringConvertibleSkipRule` value.
/// - Returns: A Boolean indicating whether the two values are equal.
public static func == (
lhs: ReflectiveStringConvertibleSkipRule,
rhs: ReflectiveStringConvertibleSkipRule
) -> Bool {
switch (lhs, rhs) {
case (.empty, .empty):
return true
case (.nilValues, .nilValues):
return true
case let (.custom(lhsIdentifier, _), .custom(rhsIdentifier, _)) where lhsIdentifier == rhsIdentifier:
return true
default:
return false
}
}
}

/// An extension for collections of `ReflectiveStringConvertibleSkipRule` values.
extension Collection where Element == ReflectiveStringConvertibleSkipRule {
/// Determines whether a given property should be skipped based on the rules
/// in the collection.
///
/// - Parameter element: A `Mirror.Child` representing the property to check.
/// - Returns: A Boolean indicating whether the property should be skipped.
func shouldBeSkipped(_ element: Mirror.Child) -> Bool {
reduce(false) { partialResult, rule in
guard !partialResult else { return partialResult }
return rule.shouldBeSkipped(element)
}
}
}

/// A protocol that extends `CustomStringConvertible` to provide reflective string conversion capabilities.
///
/// Types conforming to this protocol can customize their string representation by excluding specific properties
Expand All @@ -17,9 +115,15 @@ public protocol ReflectiveStringConvertible: CustomStringConvertible {

/// A set of property names to be transformed during the string representation.
var propertyTransformers: [String: (Any) -> String] { get }

var skipRuleSet: Set<ReflectiveStringConvertibleSkipRule> { get }
}

public extension ReflectiveStringConvertible {
var skipRuleSet: Set<ReflectiveStringConvertibleSkipRule> {
[]
}

/// The default separator used to join different parts of the string representation.
///
/// By default, this is set to a newline character ("\n").
Expand Down Expand Up @@ -71,16 +175,18 @@ public extension ReflectiveStringConvertible {
let excludedProperties = self.excludedProperties
mirror
.children
.filter { !skipRuleSet.shouldBeSkipped($0) }
.compactMap {
if let label = $0.label {
if let label = $0.label, !excludedProperties.contains(label) {
let value = propertyTransformers[label]?($0.value) ?? $0.value
return (label: label, value: value)
} else {
return nil
}
}
.filter { !excludedProperties.contains($0.label) }
.forEach { output.append(" - \($0.label): \($0.value)") }
.forEach { (child: (label: String, value: Any)) -> Void in
output.append(" - \(child.label): \(child.value)")
}

return output.joined(separator: separator)
}
Expand Down

0 comments on commit 3d73e42

Please sign in to comment.