Crash in panningEnd

[djbe]

Short description

There's a sporadic crash that we're seeing in Fabric (Crashlytics) on this line:
FloatingPanel.swift#L523

We're unable to reproduce it, and it has only happened 9 times for a total of 16k users (up to now).

Expected behavior

It shouldn't crash. 😉

Actual behavior

It crashes at the line mentioned above. From a quick look at the code, it's probably crashing because of the implicitly unwrapped optional (weak var viewcontroller: FloatingPanelController!). Changing this to a normal optional would solve the issue, and be much safer in general.

Steps to reproduce

No idea, we're only seeing the crash logs:

0  Tennis Vlaanderen              0x100eed000 closure #2 in FloatingPanel.panningEnd(with:velocity:) (FloatingPanel.swift:523)
1  Tennis Vlaanderen              0x100eed490 closure #2 in FloatingPanel.startRemovalAnimation(with:completion:) (FloatingPanel.swift:573)
2  Tennis Vlaanderen              0x100d68c3c thunk for @escaping @callee_guaranteed (@unowned UIViewAnimatingPosition) -> () (<compiler-generated>)
3  UIKitCore                      0x220640dfc -[UIViewPropertyAnimator _executeCompletionHandlerWithFinalPosition:] + 208
4  UIKitCore                      0x220640ef8 -[UIViewPropertyAnimator _runCompletions:finished:] + 124
5  UIKitCore                      0x22063fbd8 __61-[UIViewPropertyAnimator _setupAssociatedViewAnimationState:]_block_invoke + 148
6  UIKitCore                      0x22131aa7c -[UIViewAnimationBlockDelegate _didEndBlockAnimation:finished:context:] + 752
7  UIKitCore                      0x2212f17d4 -[UIViewAnimationState sendDelegateAnimationDidStop:finished:] + 312
8  UIKitCore                      0x2212f1da8 -[UIViewAnimationState animationDidStop:finished:] + 296
9  QuartzCore                     0x1f8a1b188 CA::Layer::run_animation_callbacks(void*) + 284
10 libdispatch.dylib              0x1f3f957d4 _dispatch_client_callout + 16
11 libdispatch.dylib              0x1f3f43004 _dispatch_main_queue_callback_4CF$VARIANT$mp + 1068
12 CoreFoundation                 0x1f44e6c1c __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 12
13 CoreFoundation                 0x1f44e1b54 __CFRunLoopRun + 1924
14 CoreFoundation                 0x1f44e10b0 CFRunLoopRunSpecific + 436
15 GraphicsServices               0x1f66e179c GSEventRunModal + 104
16 UIKitCore                      0x220e8f978 UIApplicationMain + 212
17 Tennis Vlaanderen              0x100c526dc main (ProfileRepository.swift:12)
18 libdyld.dylib                  0x1f3fa68e0 start + 4

Environment

Library version

1.6.0

Installation method

• CocoaPods
• Carthage
• Git submodules

iOS version(s)

11 and 12 (we only support those versions in our app)

Xcode version

10.2

[scenee]

Thanks for your detailed report!
The crash is same as #216 and the master branch has already fixed the problem 👍 I will release it on v1.6.1 in this month.

[djbe]

Nice, thanks for the fix!

Small suggestion: changing this line to an optional would prevent future issues of the same nature.

[scenee]

Small suggestion: changing this line to an optional would prevent future issues of the same nature.

I agree with you. I will improve it on prevent-found-nil-error branch. Thank you for you suggestion!

[djbe]

Small tip for that branch, you can easily chain optionals if there's no need for the unwrapped value. For example:
https://github.com/SCENEE/FloatingPanel/compare/prevent-found-nil-error#diff-78fd5332f166d7375e23becd6eceaaddR28

if let vc = viewcontroller {
    vc.delegate?.floatingPanelDidChangePosition(vc)
}

Becomes:

viewcontroller?.delegate?.floatingPanelDidChangePosition(viewcontroller)

Same for most of the other changes below, like R176, R224, R326, etc...

[scenee]

Is it right? The replacement causes the error because floatingPanelDidChangePosition(_:) needs an unwrapped FloatingPanelController value for the argument.

Value of optional type 'FloatingPanelController?' must be unwrapped to a value of type 'FloatingPanelController'

[djbe]

Ah, apologies, didn't notice you also needed to have a non-optional argument for the callbacks. In that case the code is indeed correct, you'll need to unwrap it each time.