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

Unhandled error on iOS native side: viewNotFoundForReactTag #94

Open
danielrhodes opened this issue Jan 25, 2024 · 15 comments
Open

Unhandled error on iOS native side: viewNotFoundForReactTag #94

danielrhodes opened this issue Jan 25, 2024 · 15 comments

Comments

@danielrhodes
Copy link

I am seeing these come up as unhandled errors in my exception tracker in production. I am on v 2.3 of this library and v 4.2.3 of react-native-ios-utilities. It seems to happen regardless of iOS version or device or React Native version (recently upgraded to 0.73 from 0.72).

I suspect what is happening is some sort of re-render is happening and the context menu loses its underlying reference to the root view.

Error domain: react-native-ios-utilities - code: viewNotFoundForReactTag - description: No corresponding view instance found for react tag - extraDebugValues: extraDebugValuesString: { node: 597, targetType: RNIContextMenuView } - fileName: /Users/<REDACTED>/code/<REDACTED>/packages/app/node_modules/react-native-ios-utilities/ios/Sources/Helpers/RNIModuleHelpers.swift - functionName: getView(withErrorType:forNode:type:bridge:) - lineNumber: 39 - columnNumber: 27 
    (native) construct
    (native) apply
    /Users/<REDACTED>/Library/Developer/Xcode/DerivedData/app-elacbqvdwmqsunameeuzcynrbwae/Build/Intermediates.noindex/ArchiveIntermediates/app/BuildProductsPath/Release-iphoneos/main.jsbundle:3211:28 _construct
    /Users/<REDACTED>/Library/Developer/Xcode/DerivedData/app-elacbqvdwmqsunameeuzcynrbwae/Build/Intermediates.noindex/ArchiveIntermediates/app/BuildProductsPath/Release-iphoneos/main.jsbundle:3173:25 Wrapper
    (native) construct
    /Users/<REDACTED>/Library/Developer/Xcode/DerivedData/app-elacbqvdwmqsunameeuzcynrbwae/Build/Intermediates.noindex/ArchiveIntermediates/app/BuildProductsPath/Release-iphoneos/main.jsbundle:187182:322 _createSuperInternal
    (native) call
    /Users/<REDACTED>/Library/Developer/Xcode/DerivedData/app-elacbqvdwmqsunameeuzcynrbwae/Build/Intermediates.noindex/ArchiveIntermediates/app/BuildProductsPath/Release-iphoneos/main.jsbundle:187195:26 CodedError

which references https://github.com/dominicstop/react-native-ios-utilities/blob/master/ios/Sources/Helpers/RNIModuleHelpers.swift#L38-L50

Is there anything I can do to mitigate this?

@danielrhodes danielrhodes changed the title Unhandled error on native side Unhandled error on iOS native side: viewNotFoundForReactTag Jan 25, 2024
@valentinchelle
Copy link

valentinchelle commented Jan 26, 2024

I've noticed this issue too after upgrading to 0.73 (expo 50). I've also noticed a memory leak issue (the number of views in the performance monitoring wouldn't stop going up after multiple mount unmounts). I believe that the preview isn't being unmounted correctly (i tried all the props related to garbage collection) .

Additionally, I noticed that the Auxiliary preview crashes when dismissed.

@danielrhodes
Copy link
Author

@valentinchelle I haven't been able to consistently reproduce it locally, but previously I found that it was quite vulnerable to remounts of the parent component / tree and fiddling with the cleanup may have helped somewhat.

@nandorojo
Copy link
Collaborator

Confirming we are getting the same issue:

image

@dominicstop
Copy link
Owner

dominicstop commented Feb 6, 2024

sorry for the late response, i'll take a look and start debugging - progress log for issue #94


Progress Log

  • 2024-02-06-17:08 - hi, during the refactor to expo, i moved a bunch of stuff around and accidentally broke the code that's responsible for the cleanup logic/routine.

  • 2024-02-06-17:38 - i've decided to re-write the logic that mounts the "aux. preview"/"custom preview" views + the logic that handles the cleanup routine (this might take some time) — in the meantime i'll push some temp. fixes.

  • 2024-02-06-19:44 - react-native-ios-context-menu (Release: v2.3.1 | changes)
    • Desc: Update ContextMenuView render function.

  • 2024-02-06-21:31 - react-native-ios-context-menu (Release: v2.3.2 | changes)
    • Desc: Suppress nilReactBridge and viewNotFoundForReactTag errors (i.e. silent failure).

  • 2024-02-06-21:44 - react-native-ios-utilities (Release: v4.2.4 | changes)
    • Desc: Impl. RNIDetachedView.shouldNotifyOnComponentWillUnmount, and suppress nilReactBridge error from RNIDetachedViewModule.notifyOnComponentWillUnmount.

  • 2024-02-07-02:17 - react-native-ios-utilities (Release: v4.3.0 | changes)
    • Desc: Merge changes from v4.3.0-11 and v4.2.4.

  • 2024-02-07-16:34 - react-native-ios-context-menu (Release: v2.4.0 | changes)
  • 2024-02-07-17:24 - DGSwiftUtilities (Release: 0.13.0 | changes)
    • Desc: Update HorizontalAlignmentPosition.createHorizontalConstraints.

  • 2024-02-07-17:57 - ContextMenuAuxiliaryPreview (Release: 0.4.0 | changes)
    • Desc: Raise min. dep. to [email protected], and fix exit animation bug that occurs when AuxiliaryPreviewConfig.horizontalAlignment is set to HorizontalAlignmentPosition.stretch.

  • 2024-02-08-03:57 - react-native-ios-context-menu (Release: v2.4.1 | changes)
    • Desc: Updated cleanup logic to be more aggressive.

  • 2024-02-08-15:40 - Log: Testing - Push multiple screens and pop them.
    • Observation: The number of views in the perf. monitor steadily increases, and drops once all of the screens are popped.
Simulator.Screen.Recording.-.iPhone.11.-.iOS.17.-.2024-02-08.at.15.36.59.mp4

  • 2024-02-08-15:45 - Log: Testing - Show and close the context menu example test items that have aux. previews; this is a test for RNIDetachedView cleanup.
    • Observation: The number of views in the perf. increases once the context menu + aux. preview is shown, but drops once the context menu + aux. preview is closed
Simulator.Screen.Recording.-.iPhone.11.-.iOS.17.-.2024-02-08.at.15.40.31.mp4

  • 2024-02-12-22:14 - react-native-ios-context-menu (Release: v2.5.0-0 | changes)
  • 2024-02-13-05:06 - DGSwiftUtilities (Release: 0.14.0 | changes)
    • Desc: Import DeinitialzationObserver + related sources from react-native-ios-utilities.

  • 2024-02-13-05:22 - react-native-ios-utilities (changes)
  • 2024-02-13-05:32 - react-native-ios-context-menu (Release: v2.5.0-1 | changes)
    • Desc: Update dependencies, remove RNICleanableView, and replace w/ impl. from react-native-ios-utilities.

  • 2024-02-14-03:41 - DGSwiftUtilities (changes)
  • 2024-02-15-18:57 - react-native-ios-utilities (changes)
    • Desc: Fixes for crash on production; update cleanup function, i.e. update RNIHelpers.recursivelyRemoveFromViewRegistry.
    • Releases: v4.3.1 (changes), v4.3.2 (changes)

  • 2024-02-15-22:06 - react-native-ios-context-menu (changes)
  • 2024-02-15-23:31 - react-native-ios-utilities (v4.4.0-2 | changes)
    • Desc: Impl. global flags + props to control the cleanup logic - RNIDetachedView.internalViewCleanupMode prop, update RNICleanableViewRegistry, impl. RNIUtiltiesModule + RNICleanableViewRegistryEnv.

  • 2024-02-18-09:40 - DGSwiftUtilities (changes)
  • 2024-02-20-07:11 - react-native-ios-utilities (Release: v4.4.0-3 | changes)
    • Desc: Improve cleanup logic + debugging and impl. error handling for RNICleanableView + related sources.

  • 2024-02-21-01:07 - Log: react-native-ios-utilities - Testing
    • Desc: Test cleanup logic via RNIDetachedViewTest01
Simulator.Screen.Recording.-.iPhone.11.-.iOS.17.-.2024-02-21.at.01.08.55.mp4

  • 2024-02-26-10:01 - react-native-ios-utilities (v4.4.0-4 | changes)
    • Desc: Update cleanup logic.

  • 2024-02-27-02:26 - react-native-ios-utilities (v4.4.0-5 | changes)
    • Desc: Update cleanup logic (again).

  • 2024-02-27-18:57 - react-native-ios-context-menu (Release: v2.5.0-2 | changes)
    • Desc: Update dependency for react-native-ios-utilities, and update cleanup logic for RNIContextMenuView, and RNIContextMenuButton.

  • 2024-02-27-19:05 - # Log: react-native-ios-context-menu - Testing
    • Desc: Shows the new cleanup logic.
Simulator.Screen.Recording.-.iPhone.11.-.iOS.17.-.2024-02-27.at.19.03.57.mp4



Explanation

hello, it looks like the recent patches i've made has caused a bunch of crashes; i apologize if you were affected by this.

i'll write a short explanation on why the "clean up" function is needed (hopefully, with the help of he community, we can find a solution that's reliable):

  • RCTUIManager singleton has a private variable called _viewRegistry.
    • _viewRegistry is a dictionary (i.e. NSDictionary<NSNumber, RCTView>)
    • The performance monitor is just counting the views inside the _viewRegistry.

  • Every react view has an associated reactTag: NSNumber property (which AFAIK just mirrors UIView.tag).
  • React then uses the dictionary, and the reactTag together to reference a react view in the app's view hierarchy.
    • The "react tag" is used to call methods for a specific native view instance.
    • The "react tag" used to be exposed via an API (i.e. via ReactNative.findNodeHandle), but has since been deprecated in the new architecture (and should no longer be used).
    • Currently we are getting the "react tag" via reading a private property in a native view element's ref property.
    • But it's also possible to create a onReactTagDidSet event, and pass the "react tag" value to JS.

  • The RCTUIManager._viewRegistry keeps a strong a reference to every RCTView instance to prevent them from vanishing into the void.
    • Objc/Swift uses reference counting (i.e. ARC) for memory management. This means that if there's at least one strong reference to an instance, they won't get de-referenced (unlike java, there is no GC).
    • As such it's possible for an object to be "alive" despite not being used if there something is referencing it in perpetuity, causing a memory leak.
    • Since RCTUIManager is a singleton, and will "live" for the entire duration of the app, anything inside _viewRegistry will also "live" for the entire duration of the app.

  • I'm not really sure what's the proper way to "hand off" views managed by react to UIKit, nevertheless RCTUIManager does not like it when you interfere with an RCTView's view lifecycle (e.g. moving it to a different view, removing it from it's parent, etc).
  • This is to say, once you "take" a view from RCTUIManager, it'll no longer manage the lifecycle for that specific view instance.
  • This means that it's possible for an RCTView instance to get "orphaned" and live forever.
    • Since the views are inert, they don't really use a lot of memory, since they no longer draw anything (AFAIK a view without a window isn't backed by a CALayer, as such it shouldn't be using any resources).
    • But that usage is not exactly 0 (a couple of bytes, at most); so eventually memory pressure increases due to the leak, and the app will get a low memory notification (and eventually crash).

  • Due to reasons listed above, we need a cleanup routine that "kills" the orphans.
  • I've been using this helper function to purge the orphans (and their descendants) from the registry, so that the strong reference is removed, and they can be free.
  • The problem is timing, e.g. when to trigger that cleanup routine.
  • You have to be sure that the react view you want to "clean" is no longer being used, otherwise if something tries to ref. that view (e.g. like a reanimated worklet), a crash will occur.
  • I've been mostly relying on onComponentWillUnmount, and deinit to trigger the cleanup routine.
  • Proposed solution: Diagram-RNIViewCleanupMode-Export-2024-02-12-23-42-41.pdf

@alantoa
Copy link
Contributor

alantoa commented Feb 7, 2024

I encountered this issue before, and it was fixed by react-native-ios-context-menu v2.4.0 and react-native-ios-utilities v4.3.0 on my end.
Could you guys try it out?

@danielrhodes
Copy link
Author

danielrhodes commented Feb 8, 2024

I updated but this is still occurring for me, but I might have a better clue about what is happening:

I am using react-navigation, and if I click a menu item which triggers a screen change, it might cause this error. I believe this is because the menu has not closed by the time the view is removed.

I have no idea if this will fix it, but I am putting in a setTimeout after receiving an event - will see if that helps.

@dominicstop
Copy link
Owner

dominicstop commented Feb 8, 2024

@danielrhodes hi, i need some sort of small repro/code snippet so i can debug this further (i'll add it as a test case in the example app for future reference). i'm having some trouble trying to recreate the issues you're describing.

if possible, can you please provide some screenshots/video of the performance monitor while re-creating the issue/bug? (you can crop the video via the photos app to redact the contents of your app)

(i've added some screen recordings in my previous reply)

@danielrhodes
Copy link
Author

@dominicstop I haven't been able to replicate it locally, but I am seeing it quite often from exception logs in production.

Here is an example project that isn't much different from my own to give you something to work with. https://github.com/danielrhodes/ios-context-menu-bug-example

@alantoa
Copy link
Contributor

alantoa commented Feb 12, 2024

yeah, we only found this error log in the production log. but so far I have discovered that this error has not caused the app to crash yet. so it's kind of weird...

@dominicstop
Copy link
Owner

dominicstop commented Feb 21, 2024

@danielrhodes it looks like the view count in the performance monitor is not up to date somehow (there is no memory leak)

Screenshot 2024-02-21 at 11 02 32 PM



Screenshot 2024-02-21 at 11 21 17 PM



Screenshot 2024-02-21 at 11 14 04 PM



Screenshot 2024-02-21 at 11 20 35 PM

@danielrhodes
Copy link
Author

@danielrhodes it looks like the view count in the performance monitor is not up to date somehow (there is no memory leak)

If that's using the project I shared above, this could be because FlashList does view recycling. I'm not too familiar with how this all works under the hood, but one thing I've wondered is if this error happens while a re-render is happens and the context menu is still displaying.

@dominicstop
Copy link
Owner

dominicstop commented Feb 26, 2024

@danielrhodes hello, i've done some more research...

i look at the code for RCTPerformanceManager, and it looks like it just counts the views from the view registry....

2024-02-26-11-21-15

after that, i started debugging again...

  • removed all the views in the JS-side via conditional rendering
  • fired up the memory graph; it's saying there's 11 RCTView instances in memory
  • read RCTUIManager._viewRegistry, it's saying there's 1584 items...
  • But all the ref. were held by REAUIManager from react-native-reanimated? (but i'm pretty sure it's not related to the issue)
    2024-02-26-11-07-40



  • strangely enough, it seems that if you get the RCTBridge.uiManager, the instance it returns is REAUIManager
  • the screenshot belows shows that RCTUIManager and REAUIManager have the same memory address (i just thought it was interesting, but probably unrelated ashsdhkllsh)
2024-02-26-12-06-20 2024-02-26-12-16-03

EDIT: I think i might finally fixed this in the latest prerelease version.

Simulator.Screen.Recording.-.iPhone.11.-.iOS.17.-.2024-02-27.at.04.21.57.mp4

@nandorojo
Copy link
Collaborator

Interestingly, I don't think I've had this issue since upgrading to 2.4.2 and ios-utilities 4.3.1

@danielrhodes
Copy link
Author

EDIT: I think i might finally fixed this in the latest prerelease version.

Nice! When the release comes out, I can cut new release to try it.

@dominicstop
Copy link
Owner

dominicstop commented Feb 27, 2024

hello @danielrhodes

for now you would have to use: [email protected] + [email protected].

i'm still testing all of the example + test items in the example app; i just wanna be sure first that the changes i've made recently won't cause a crash somehow (i haven't found any issues so far...)

it would be great if you could try it out, and see if there are any bugs + give feedback ahsdjhkdsldfkdj

if you decide to install the pre-release version, i recommend adding the code snippet below in you App.js/app entry file so we can get some more debugging info (you can disable/remove them anytime).

import { setSharedEnvForRNICleanableViewRegistry, setSharedEnvForRNIUtilitiesModule } from 'react-native-ios-utilities';

setSharedEnvForRNIUtilitiesModule({
  debugShouldLogViewRegistryEntryRemoval: true,
  overrideEnableLogStackTrace: true,
  overrideShouldLogFileMetadata: true,
  overrideShouldLogFilePath: true
});

setSharedEnvForRNICleanableViewRegistry({
  debugShouldLogCleanup: true,
  debugShouldLogRegister: true,
  shouldGloballyDisableCleanup: false,
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants