Skip to content
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

Add root error info to public error #4680

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions Sources/Error Handling/ErrorDetails.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ extension NSError.UserInfoKey {
static let attributeErrorsResponse: NSError.UserInfoKey = "attributes_error_response"
static let statusCode: NSError.UserInfoKey = "rc_response_status_code"
static let obfuscatedEmail: NSError.UserInfoKey = "rc_obfuscated_email"
static let rootError: NSError.UserInfoKey = "rc_root_error"

static let readableErrorCode: NSError.UserInfoKey = "readable_error_code"
static let backendErrorCode: NSError.UserInfoKey = "rc_backend_error_code"
Expand All @@ -35,6 +36,7 @@ enum ErrorDetails {
static let attributeErrorsResponseKey = NSError.UserInfoKey.attributeErrorsResponse as String
static let statusCodeKey = NSError.UserInfoKey.statusCode as String
static let obfuscatedEmailKey = NSError.UserInfoKey.obfuscatedEmail as String
static let rootErrorKey = NSError.UserInfoKey.rootError as String

static let readableErrorCodeKey = NSError.UserInfoKey.readableErrorCode as String
static let extraContextKey = NSError.UserInfoKey.extraContext as String
Expand Down
64 changes: 63 additions & 1 deletion Sources/Error Handling/PurchasesError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// Created by Nacho Soto on 8/31/22.

import Foundation
import StoreKit

/// An error returned by a `RevenueCat` public API.
public typealias PublicError = NSError
Expand Down Expand Up @@ -39,7 +40,29 @@ extension PurchasesError {
/// let errorCode = error as? ErrorCode
/// ```
var asPublicError: PublicError {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main concern with this approach is whether we only expose PublicError everywhere and/or if we use this method somewhere else... Need to research it a bit more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's a draft, but it would be great to list the keys we'll include in userInfo so people can just read it from here without going through the code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm so I included it in the docs for the asPublicError, but that's actually internal... I'm trying to figure out a better way to explain this, but maybe this is something we should add to the docs, for people that care to get the root error.

return NSError(domain: Self.errorDomain, code: self.errorCode, userInfo: self.userInfo)
let rootError: Error = self.rootError(from: self)
let rootNSError = rootError as NSError
var rootErrorInfo = [
"code": rootNSError.code,
"domain": rootNSError.domain,
"localizedDescription": rootNSError.localizedDescription,
] as [String : Any]
if #available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, visionOS 1.0, *) {
if let storeKitErrorInfo = self.getStoreKitErrorInfoIfAny(error: rootError) {
rootErrorInfo = rootErrorInfo.merging(["storeKitError": storeKitErrorInfo])
}
}
let userInfoToUse = self.userInfo.merging([ErrorDetails.rootErrorKey: rootErrorInfo])
return NSError(domain: Self.errorDomain, code: self.errorCode, userInfo: userInfoToUse)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid we might lose some keys because of an unwanted overlap. I would just add a prefix like rc_ to any new key we add to:

  • be explicit on what we're adding
  • avoid dropping useful data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I added everything within a rc_root_error which I felt was enough as a namespace... I don't think it's worth adding to all nested properties then? Lmk if you think otherwise though!

}

private func rootError(from error: Error) -> Error {
let nsError = error as NSError
if let underlyingError = nsError.userInfo[NSUnderlyingErrorKey] as? Error {
return rootError(from: underlyingError)
} else {
return error
}
}

}
Expand All @@ -65,3 +88,42 @@ extension PurchasesError {
}

}

@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, visionOS 1.0, *)
private extension PurchasesError {

func getStoreKitErrorInfoIfAny(error: Error) -> [String: Any]? {
if let skError = error as? SKError {
return [
"skErrorCode": skError.code,
"description": skError.code.trackingDescription
]
} else if let storeKitError = error as? StoreKitError {
let resultMap: [String: Any] = ["description": storeKitError.trackingDescription]
switch storeKitError {
case .unknown,
.userCancelled,
.notAvailableInStorefront,
.notEntitled:
return resultMap
case let .networkError(urlError):
return resultMap.merging([
"urlErrorCode": urlError.errorCode,
"urlErrorFailingUrl": urlError.failureURLString ?? ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe we can add an explicit missing_value, otherwise people will think we missed it

])
case let .systemError(systemError):
return resultMap.merging([
"systemErrorDescription": systemError.localizedDescription
])

@unknown default:
return resultMap
}
} else if let storeKitError = error as? StoreKit.Product.PurchaseError {
return ["description": storeKitError.trackingDescription]
} else {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would be useful to log what other error type we might be missing? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, in this case I'm assuming it's because the root error is not a store kit error (which may happen in several situations). I could add a log here, but I think the situation may happen relatively often... Note that this method is only to add extra data to the userInfo map in case it's actually a StoreKit error.

}
}

}