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

feat: Swift error names #2960

Merged
merged 14 commits into from
May 3, 2023
21 changes: 21 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,27 @@

## Unreleased

- Swift Error Names (#2960)

```Swift
enum LoginError: Error {
case wrongUser(id: String)
case wrongPassword
}

SentrySDK.capture(error: LoginError.wrongUser("12345678"))
```

For the Swift error above Sentry displays:

| sentry-cocoa SDK | Title | Description |
| ----------- | ----------- | ----------- |
| Since 8.7.0 | `LoginError` | `wrongUser(id: "12345678") (Code: 1)` |
| Before 8.7.0 | `LoginError` | `Code: 1` |

[Customized error descriptions](https://docs.sentry.io/platforms/apple/usage/#customizing-error-descriptions) have precedence over this feature.
This change has no impact on grouping of the issues in Sentry.

### Fixes

- Propagate span when copying scope (#2952)
Expand Down
17 changes: 0 additions & 17 deletions Samples/iOS-Swift/iOS-Swift/Tools/RandomErrors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,6 @@ enum SampleError: Error {
case awesomeCentaur
}

extension SampleError: CustomNSError {
var errorUserInfo: [String: Any] {
func getDebugDescription() -> String {
switch self {
case SampleError.bestDeveloper:
return "bestDeveloper"
case .happyCustomer:
return "happyCustomer"
case .awesomeCentaur:
return "awesomeCentaur"
}
}

return [NSDebugDescriptionErrorKey: getDebugDescription()]
}
}

class RandomErrorGenerator {

static func generate() throws {
Expand Down
14 changes: 14 additions & 0 deletions Sources/Sentry/SentryClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#import "SentrySDK+Private.h"
#import "SentryScope+Private.h"
#import "SentryStacktraceBuilder.h"
#import "SentrySwift.h"
#import "SentryThreadInspector.h"
#import "SentryTraceContext.h"
#import "SentryTracer.h"
Expand Down Expand Up @@ -248,9 +249,22 @@ - (SentryEvent *)buildErrorEvent:(NSError *)error

// If the error has a debug description, use that.
NSString *customExceptionValue = [[error userInfo] valueForKey:NSDebugDescriptionErrorKey];

NSString *swiftErrorDescription = nil;
// SwiftNativeNSError is the subclass of NSError used to represent bridged native Swift errors,
// see
// https://github.com/apple/swift/blob/067e4ec50147728f2cb990dbc7617d66692c1554/stdlib/public/runtime/ErrorObject.mm#L63-L73
NSString *errorClass = NSStringFromClass(error.class);
if ([errorClass containsString:@"SwiftNativeNSError"]) {
swiftErrorDescription = [SwiftDescriptor getSwiftErrorDescription:error];
}

if (customExceptionValue != nil) {
exceptionValue =
[NSString stringWithFormat:@"%@ (Code: %ld)", customExceptionValue, (long)error.code];
} else if (swiftErrorDescription != nil) {
exceptionValue =
[NSString stringWithFormat:@"%@ (Code: %ld)", swiftErrorDescription, (long)error.code];
} else {
exceptionValue = [NSString stringWithFormat:@"Code: %ld", (long)error.code];
}
Expand Down
5 changes: 5 additions & 0 deletions Sources/Swift/SwiftDescriptor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,9 @@ public class SwiftDescriptor: NSObject {
return String(describing: type(of: object))
}

@objc
public static func getSwiftErrorDescription(_ error: Error) -> String? {
return String(describing: error)
}
Comment on lines +12 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

h: Why are we not concern with PII anymore?

Copy link
Member Author

@philipphofmann philipphofmann May 3, 2023

Choose a reason for hiding this comment

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

I guess you missed my update in the description above #2960 (comment)

Update: No need to worry about PII in a major as we already would send PII of Errors with

// The description of the error can be especially useful for error from swift that
// use a simple enum.
mechanism.desc = error.description;

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 you missed my update in the description above

Yeap I did. I would like to suggest to only update description if something is wrong, change of direction in the PR should be describe in a new comment. Or both, since GH itself don't warn about description changing.

Thanks for clarifying!


}
84 changes: 81 additions & 3 deletions Tests/SentryTests/SentryClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ class SentryClientTest: XCTestCase {
eventId.assertIsNotEmpty()
let error = TestError.invalidTest as NSError
assertLastSentEvent { actual in
assertValidErrorEvent(actual, error)
assertValidErrorEvent(actual, error, exceptionValue: "invalidTest (Code: 0)")
}
}

Expand Down Expand Up @@ -456,6 +456,62 @@ class SentryClientTest: XCTestCase {
}
}
}

func testCaptureSwiftError_UsesSwiftStringDescription() {
let eventId = fixture.getSut().capture(error: SentryClientError.someError)

eventId.assertIsNotEmpty()
assertLastSentEvent { actual in
do {
let exceptions = try XCTUnwrap(actual.exceptions)
XCTAssertEqual("someError (Code: 1)", try XCTUnwrap(exceptions.first).value)
} catch {
XCTFail("Exception expected but was nil")
}
}
}

func testCaptureSwiftErrorStruct_UsesSwiftStringDescription() {
let eventId = fixture.getSut().capture(error: XMLParsingError(line: 10, column: 12, kind: .internalError))

eventId.assertIsNotEmpty()
assertLastSentEvent { actual in
do {
let exceptions = try XCTUnwrap(actual.exceptions)
XCTAssertEqual("XMLParsingError(line: 10, column: 12, kind: SentryTests.XMLParsingError.ErrorKind.internalError) (Code: 1)", try XCTUnwrap(exceptions.first).value)
} catch {
XCTFail("Exception expected but was nil")
}
}
}

func testCaptureSwiftErrorWithData_UsesSwiftStringDescription() {
let eventId = fixture.getSut().capture(error: SentryClientError.invalidInput("hello"))

eventId.assertIsNotEmpty()
assertLastSentEvent { actual in
do {
let exceptions = try XCTUnwrap(actual.exceptions)
XCTAssertEqual("invalidInput(\"hello\") (Code: 0)", try XCTUnwrap(exceptions.first).value)
} catch {
XCTFail("Exception expected but was nil")
}
}
}

func testCaptureSwiftErrorWithDebugDescription_UsesDebugDescription() {
let eventId = fixture.getSut().capture(error: SentryClientErrorWithDebugDescription.someError)

eventId.assertIsNotEmpty()
assertLastSentEvent { actual in
do {
let exceptions = try XCTUnwrap(actual.exceptions)
XCTAssertEqual("anotherError (Code: 0)", try XCTUnwrap(exceptions.first).value)
} catch {
XCTFail("Exception expected but was nil")
}
}
}

func testCaptureErrorWithComplexUserInfo() {
let url = URL(string: "https://github.com/getsentry")!
Expand Down Expand Up @@ -1458,7 +1514,7 @@ class SentryClientTest: XCTestCase {
}
}

private func assertValidErrorEvent(_ event: Event, _ error: NSError) {
private func assertValidErrorEvent(_ event: Event, _ error: NSError, exceptionValue: String? = nil) {
XCTAssertEqual(SentryLevel.error, event.level)
XCTAssertEqual(error, event.error as NSError?)

Expand All @@ -1469,7 +1525,7 @@ class SentryClientTest: XCTestCase {
let exception = exceptions[0]
XCTAssertEqual(error.domain, exception.type)

XCTAssertEqual("Code: \(error.code)", exception.value)
XCTAssertEqual(exceptionValue ?? "Code: \(error.code)", exception.value)

XCTAssertNil(exception.threadId)
XCTAssertNil(exception.stacktrace)
Expand Down Expand Up @@ -1559,4 +1615,26 @@ class SentryClientTest: XCTestCase {

}

enum SentryClientError: Error {
case someError
case invalidInput(String)
}

enum SentryClientErrorWithDebugDescription: Error {
case someError
}

extension SentryClientErrorWithDebugDescription: CustomNSError {
var errorUserInfo: [String: Any] {
func getDebugDescription() -> String {
switch self {
case .someError:
return "anotherError"
}
}

return [NSDebugDescriptionErrorKey: getDebugDescription()]
}
}

// swiftlint:enable file_length
41 changes: 41 additions & 0 deletions Tests/SentryTests/SwiftDescriptorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,48 @@ class SwiftDescriptorTests: XCTestCase {
XCTAssertEqual(name, "InnerClass")
}

func testGetSwiftErrorDescription_EnumValue() {
let actual = SwiftDescriptor.getSwiftErrorDescription(LoginError.wrongPassword)
XCTAssertEqual("wrongPassword", actual)
}

func testGetSwiftErrorDescription_EnumValueWithData() {
let actual = SwiftDescriptor.getSwiftErrorDescription(LoginError.wrongUser(name: "Max"))
XCTAssertEqual("wrongUser(name: \"Max\")", actual)
}

func testGetSwiftErrorDescription_StructWithData() {
let actual = SwiftDescriptor.getSwiftErrorDescription(XMLParsingError(line: 10, column: 12, kind: .internalError))
XCTAssertEqual("XMLParsingError(line: 10, column: 12, kind: SentryTests.XMLParsingError.ErrorKind.internalError)", actual)
}

func testGetSwiftErrorDescription_StructWithOneParam() {
let actual = SwiftDescriptor.getSwiftErrorDescription(StructWithOneParam(line: 10))
XCTAssertEqual("StructWithOneParam(line: 10)", actual)
}

private func sanitize(_ name: AnyObject) -> String {
return SwiftDescriptor.getObjectClassName(name)
}
}

enum LoginError: Error {
case wrongUser(name: String)
case wrongPassword
}

struct XMLParsingError: Error {
enum ErrorKind {
case invalidCharacter
case mismatchedTag
case internalError
}

let line: Int
let column: Int
let kind: ErrorKind
}

struct StructWithOneParam: Error {
let line: Int
}