`model(at:) thows -> Any` method doesn't throw.

[cameroncooke]

Short description of the issue:

The model(at:) convenience method of CollectionViewSectionedDataSource is marked as throwing yet the implementation doesn't throw and can't throw as the underlying logic doesn't handle errors by throwing. There are no try statements within the body of the method.

https://github.com/ReactiveX/RxSwift/blob/ef8b0abe3334c5853001577125c82c86c55b41ce/RxExample/RxDataSources/RxDataSources/CollectionViewSectionedDataSource.swift#L78

The presence of the throws in the API contract caused us to introduce a production crash in our app because we believed that this method would throw when subscript was out of bounds which we handled at the callsite using try?. Instead as this method doesn't actually throw the underlying subscript implementation raises a fatalError instead.

Expected outcome:

Method to not be marked as throwing if it doesn't throw.

What actually happens:

Nothing directly, but in the case that a subscript out of bounds error is raised it's not handled by RxSwift as the throws method might suggest which can introduce bugs in the application using RxSwift.

Self contained code example that reproduces the issue:

func doSomething() {
    let outOfBoundsIndexPath = IndexPath(item: 1000, section: 0)
    guard let model = try? datasource.model(at: outOfBoundsIndexPath) else { return nil } // crashes instead of being handled
}

RxSwift/RxDataSources versions

5.1.1/4.0.1

Platform/Environment

• [x] iOS
• [ ] macOS
• [ ] tvOS
• [ ] watchOS
• [ ] playgrounds

How easy is to reproduce? (chances of successful reproduce after running the self contained code)

• [x] easy, 100% repro
• [ ] sometimes, 10%-100%
• [ ] hard, 2% - 10%
• [ ] extremely hard, %0 - 2%

Xcode version:

Version 12.1 (12A7403)

Installation method:

• [x] SPM
• [ ] CocoaPods
• [ ] Carthage
• [ ] Git submodules

I have multiple versions of Xcode installed: (so we can know if this is a potential cause of your issue)

• [ ] yes (which ones)
• [x] no

Level of RxSwift knowledge: (this is so we can understand your level of knowledge and formulate the response in an appropriate manner)

• [ ] just starting
• [ ] I have a small code base
• [x] I have a significant code base

[freak4pc]

That's a very interesting crash. Thank you for reporting! The real issue is that I'm not sure if this method needs to throw at all, or if it should throw an optional. The latter is a breaking API change but we're also releasing a new version soon, so I'm a bit ambivalent to this. Any thoughts? The first is easier, of course.

[cameroncooke]

Well it doesn't have the ability to throw as the method body doesn't call any throwing functions or throw itself. The 'try' here is completely redundant and just confuses the API contract. But you're right removing the redundant try would be a breaking change. I guess you could check indices of the models array and throw an error if a index doesn't exist? That is actual what we've had to do in our use-case and would give the 'try' a reason to exist. Though none of Apple's subscript or convenience APIs throw and instead just fatalError.

Alternatively I would just add api documentation that this method doesn't actually throw.

[freak4pc]

I guess you could check indices of the models array and throw an error if a index doesn't exist?

Yeah this is obviously what needs to be done it we don't break the contract. I'm saying that we can theoretically break this contract because we're bumping a major version soon. It might make more sense to make it return an optional and be done with it... ?

[oteronavarretericardo]

+1 for throwing. The source code seems pretty intentional about try use. In particular the return value of model(at:) is generic T and passed in as argument to castOrFatalError() -> T. Having T? instead of T would make this cast difficult.

[freak4pc]

I just realized this code is part of the example project and not part of RxSwift itself at all, I'm not sure if there's direct value to fixing it here. Sounds like the fix should be in RxDataSources.

[freak4pc]

I've made a copy of this in the appropriate repo and I'll address it there in the next release. Thanks! https://github.com/RxSwiftCommunity/RxDataSources/issues/388