Skip to content

Commit

Permalink
fix: do not force cast KVO values (#359)
Browse files Browse the repository at this point in the history
## 📜 Description

Do not use force casting in swift - replaced with optional casting and
guard statements.

## 💡 Motivation and Context

In sentry one of library user seen this:

```bash
Thread 0 - (TH_STATE_WAITING)
0  Trusted +0x5746c0        Swift runtime failure: Unexpectedly found nil while unwrapping an Optional value (KeyboardMovementObserver.swift:109:65)
1  Trusted +0x5705cc        KeyboardMovementObserver.observeValue(forKeyPath:of:change:context:) (<compiler-generated>)
2  Foundation +0x99e54      _NSKeyValueNotifyObserver
3  Foundation +0x99cf0      _NSKeyValueDidChange
4  Foundation +0x99b20      -[NSObject(NSKeyValueObservingPrivate) _changeValueForKeys:count:maybeOldValuesDict:maybeNewValuesDict:usingBlock:]
5  Foundation +0x99850      -[NSObject(NSKeyValueObservingPrivate) _changeValueForKey:key:key:usingBlock:]
6  Foundation +0x12cb94     __NSSetPointValueAndNotify
7  UIKitCore +0x615f4       -[UIView(Geometry) _applyISEngineLayoutValuesToBoundsOnly:]
8  UIKitCore +0x60e44       -[UIView(Geometry) _resizeWithOldSuperviewSize:]
9  CoreFoundation +0x353f0  ___NSARRAY_IS_CALLING_OUT_TO_A_BLOCK__
```

It was very random crash, but still - it's not a good practice to use
force casting and as a prof -> it produces a crash in certain cases.

So in this PR I replaced it with more safe operators.

Closes
#93 (comment)

## 📢 Changelog

### iOS

- do not force cast KVO values

## 🤔 How Has This Been Tested?

There is no way to test whether crashes are gone or not, but I think
they should gone (in worst case we will have a new stacktrace).

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
  • Loading branch information
kirillzyusko authored Feb 19, 2024
1 parent ce4e4e7 commit 031c64c
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
3 changes: 1 addition & 2 deletions ios/observers/FocusedInputObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,7 @@ public class FocusedInputObserver: NSObject {
change _: [NSKeyValueChangeKey: Any]?,
context _: UnsafeMutableRawPointer?
) {
// swiftlint:disable:next force_cast
if keyPath == "center", object as! NSObject == currentInput! {
if keyPath == "center", object as? NSObject == currentInput {
// we need to read layout in next frame, otherwise we'll get old
// layout values
DispatchQueue.main.async {
Expand Down
9 changes: 5 additions & 4 deletions ios/observers/KeyboardMovementObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,7 @@ public class KeyboardMovementObserver: NSObject {
change: [NSKeyValueChangeKey: Any]?,
context _: UnsafeMutableRawPointer?
) {
// swiftlint:disable:next force_cast
if keyPath == "center", object as! NSObject == _keyboardView! {
if keyPath == "center", object as? NSObject == _keyboardView {
// if we are currently animating keyboard -> we need to ignore values from KVO
if displayLink != nil {
return
Expand All @@ -117,8 +116,10 @@ public class KeyboardMovementObserver: NSObject {
return
}

// swiftlint:disable:next force_cast
let keyboardFrameY = (change?[.newKey] as! NSValue).cgPointValue.y
guard let changeValue = change?[.newKey] as? NSValue else {
return
}
let keyboardFrameY = changeValue.cgPointValue.y
let keyboardWindowH = keyboardView?.window?.bounds.size.height ?? 0
let keyboardPosition = keyboardWindowH - keyboardFrameY
let position = CGFloat.interpolate(
Expand Down

0 comments on commit 031c64c

Please sign in to comment.