Skip to content

Cannot subclass User any more #5079

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.

Sign up for GitHub

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

Jump to bottom
Closed
DarkDust opened this issue Apr 10, 2025 · 7 comments · May be fixed by getsentry/sentry-docs#13509
Closed

Cannot subclass User any more #5079

DarkDust opened this issue Apr 10, 2025 · 7 comments · May be fixed by getsentry/sentry-docs#13509
Assignees
@philipphofmann
Labels
Bug Platform: Cocoa

Comments

@DarkDust
Copy link

Platform

macOS

Environment

Production

Installed

CocoaPods

Version

8.49.0

Xcode Version

16.2

Did it work on previous versions?

8.41.0

Steps to Reproduce

We have a subclass of Sentry.User with additional informations. Due to the addition of the Codable support in a recent version of the Sentry SDK, this subclass cannot be used any more. The Swift compiler complains:

'required' initializer 'init(from:)' must be provided by subclass of 'User'

However, we cannot provide it, either, since then Swift complains:

initializer 'init(from:)' is declared in extension of 'User' and cannot be overridden

Code to reproduce:

class MyUser: Sentry.User {
	let moreInfo: String
	
	init(userId: String, moreInfo: String) {
		self.moreInfo = moreInfo
		
		super.init(userId: userId)
	}
	
	required init() {
		fatalError("init() has not been implemented")
	}
	
	@available(*, deprecated, message: "…")
	required convenience init(from decoder: any Decoder) throws {
		fatalError("init(from:) has not been implemented")
	}
}

Expected Result

Existing subclass of User should still compile.

Actual Result

We were able to work around this problem by using a convenience initializer, but lost the ability to provide additional informations.

As far as I can see, the only real solution would be to port the whole User class to Swift and have the codable implementation in the main class instead of an extension:

@objc(SentryUser)
class User: NSObject, SentrySerializable, NSCopying, Codable {
   …
}

This way subclasses are able to override the decoding initializer.

Are you willing to submit a PR?

maybe
@github-project-automation github-project-automation bot moved this to Needs Discussion in [DEPRECATED] Mobile SDKs Apr 10, 2025
@philprime
Copy link
Contributor

Hi @DarkDust, thanks for raising this issue.

I do not think that it was intended that SentryUser is to be subclassed, but maybe @philipphofmann can share some insights on that.

Did you try wrap the SentryUser instead of subclassing it?

@DarkDust
Copy link
Author

No, I just added a convenience initializer to User and ditched the additional info since it wasn't used right now anyway. So it's not a big deal for us that it cannot be subclassed any more, but it was still surprising since our existing code didn't compile any more after updating the Sentry SDK.

@philprime
Copy link
Contributor

@DarkDust that is of course unintended and unfortunate, as we usually try for that not to happen.

@philipphofmann
Copy link
Member

Ahh, sorry, that's an unfortunate side effect of adding Codable via Swift extensions that I totally missed when adding the implementation. @DarkDust, do you have a workaround? We must investigate if we can somehow fix this, but my hopes aren't high.

@DarkDust
Copy link
Author

@philipphofmann: I guess the only real solution is to port User to Swift, like this:

@objc(SentryUser)
class User: NSObject, SentrySerializable, NSCopying, Codable {
   …
}

The important part is Codable being part of the class definition, not an extension. This way subclasses are able to override the required initializer of Decodable.

@philipphofmann philipphofmann moved this from Needs Discussion to Todo in [DEPRECATED] Mobile SDKs Apr 16, 2025
@philipphofmann philipphofmann self-assigned this Apr 16, 2025
@philipphofmann
Copy link
Member

We should be able to fix this by adding extra subclasses for the decodable implementations similar to what we did for SentryEvent

sentry-cocoa/Sources/Sentry/Public/SentryEvent.h

Lines 199 to 217 in 4c5c48c

/**
* Subclass of SentryEvent so we can add the Decodable implementation via a Swift extension. We need
* this due to our mixed use of public Swift and ObjC classes. We could avoid this class by
* converting SentryReplayEvent back to ObjC, but we rather accept this tradeoff as we want to
* convert all public classes to Swift in the future. This class needs to be public as we can't add
* the Decodable extension implementation to a class that is not public.
*
* @note: We can’t add the extension for Decodable directly on SentryEvent, because we get an error
* in SentryReplayEvent: 'required' initializer 'init(from:)' must be provided by subclass of
* 'Event' Once we add the initializer with required convenience public init(from decoder: any
* Decoder) throws { fatalError("init(from:) has not been implemented")
* }
* we get the error initializer 'init(from:)' is declared in extension of 'Event' and cannot be
* overridden. Therefore, we add the Decodable implementation not on the Event, but to a subclass of
* the event.
*/
@interface SentryEventDecodable : SentryEvent
@end

Ideally, we add these classes in Swift and make them internal.

@philipphofmann philipphofmann moved this from Todo to In Progress in [DEPRECATED] Mobile SDKs Apr 25, 2025
@philipphofmann philipphofmann mentioned this issue Apr 25, 2025
Open
6 tasks
@philipphofmann
Copy link
Member

I again looked into this, and adding all these subclasses increases the SDK size and the complexity of our decoable Swift code. While strictly speaking this is a breaking change, the affected classes aren't meant to be subclassed anyways and there is an easy workaround to solve the problem. I added an entry to the troubleshooting guide getsentry/sentry-docs#13509. We can revisit this decision if more people come up with good use cases for subclassing such classes.

@philipphofmann philipphofmann moved this from In Progress to Needs Review in [DEPRECATED] Mobile SDKs Apr 25, 2025
@philipphofmann philipphofmann closed this as not planned Won't fix, can't repro, duplicate, stale Apr 25, 2025
@github-project-automation github-project-automation bot moved this from Needs Review to Done in [DEPRECATED] Mobile SDKs Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@philipphofmann philipphofmann

Labels
Bug Platform: Cocoa
Projects
[DEPRECATED] Mobile SDKs
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(apple): Troubleshooting for subclass error getsentry/sentry-docs
4 participants
@DarkDust @philipphofmann @stephanie-anderson @philprime