apptentive / apptentive-ios

Apptentive Legacy SDK for iOS. See https://github.com/apptentive/apptentive-kit-ios for Version 6.
http://www.apptentive.com/
BSD 3-Clause "New" or "Revised" License
146 stars 103 forks source link

Failed to display a survey on iOS 14 Beta. App crash. #287

Closed mike-dutka closed 1 year ago

mike-dutka commented 4 years ago

Device: iPad Pro 9.7in iOS 14 Beta (18A5351d)

App fails to show a survey after Apptentive.shared.engage(event: eventName, withCustomData: nil, from: controller) than stops responding and crashes in a minute. Profiling shows that there is some problem with an Apptentive collection layout flow which leads to huge memory consumption and app crashing.

Screenshot 2020-08-26 at 16 13 56 Screenshot 2020-08-26 at 16 21 56
CaseyApptentive commented 4 years ago

Hi @mike-dutka. Thanks for the report. We'll take a closer look as soon as possible.

What version of the SDK are you using?

mike-dutka commented 4 years ago

Hi @CaseyApptentive. Tried 5.2.12 and 5.2.14

CaseyApptentive commented 4 years ago

Thanks for the versions, @mike-dutka !

We're able to reproduce the issue now and will let you know as soon as we have a fix. This is a priority for us and we'll have it available as soon as we can have everything verified.

frankus commented 4 years ago

This is fixed on the new ios-14 branch released today. Please take a look and let us know if it works for you.

Thanks again for the detailed report!

We plan to merge these fixes into the main branch and cut a release as soon as the iOS 14 GM seed drops.

braker1nine commented 4 years ago

@frankus we're having issues with the changes to the header imports when we pull this branch in via cocoapods. Our builds are failing because some header files aren't found.

Changing these imports in the Apptentive code 👇

#import <Apptentive/ApptentiveStyleSheet.h>

to this

#import <ApptentiveStyleSheet.h>

resolves the issue

Has anyone else experienced this? I'm trying to figure out if it's something in our cocoapods setup that's wrong

frankus commented 4 years ago

I am able to reproduce this by removing the use_frameworks! directive from the Podfile, so if you're able to use that directive, that should fix the problem.

It's been a few years since I've investigated this, but it sounds like use_frameworks! has fallen out of favor somewhat due to longer app startup times. Can you tell me a bit more about your setup so we can look at improving support for this? Mostly around language mix (Swift/Obj-C) and how you're actually importing things (e.g. Bridging Header).

braker1nine commented 4 years ago

Ahhh right right. Our codebase is majority Objective-C with a slowly increasing chunk of swift. Our actual use of the Apptentive SDK is all in a single Objective-C wrapper where we're importing the root Apptentive header file with an #import "Apptentive.h"

frankus commented 4 years ago

We've just released the changes as version 5.3.0.

If adding the use_frameworks! directive to your podfile is not something you're comfortable with, there are instructions here to add a post-install hook and a preprocessor macro that should fix the include issues.

If you can use use_frameworks! then it should be smooth sailing without any extra work.

braker1nine commented 4 years ago

Bummer. That's probably not a change we want to make just to support a single package. I'd definitely say I'd recommend y'all figure out how to get the library building without requiring it to be a framework. But admittedly I don't have a full grasp of the challenges there. And I know we're all on a tight timeline with iOS 14 😅

For the long term we may either build more manually or maybe use a git submodule

frankus commented 4 years ago

@braker1nine In 5.3.1 we've updated the way headers are managed, so you should be able to include it as you have been all along. The only change you might notice is that the Apptentive.h header is no longer copied, so you'll have to include the two headers that live under that umbrella header manually (ApptentiveMain.h and ApptentiveStyleSheet.h).

braker1nine commented 4 years ago

@frankus I saw that this morning and already have a branch in our codebase building off that branch! 👍 thanks we really appreciate the quick turnaround!