braze-inc / braze-swift-sdk

Braze SDK for the Apple ecosystem, including: iOS, macOS, iPadOS, visionOS, tvOS
https://www.braze.com
Other
49 stars 19 forks source link

[Bug]: Braze not loading resources/Localization #44

Closed Oaseru17 closed 1 year ago

Oaseru17 commented 1 year ago

Platform

iOS

Platform Version

iO 15.5

Braze SDK Version

5.10.0

Xcode Version

14.1

Computer Processor

Apple (M1)

Repro Rate

100%

Steps To Reproduce

Step up

  1. Braze.Configuration(.. with the required inputs)

Expected Behavior

Braze can find resources

Actual Incorrect Behavior

[braze] Error: Unable to find resources bundle, cannot load localizations and acknowledgments

Verbose Logs

No response

Additional Information

No response

lowip commented 1 year ago

Hi @Oaseru17,

Could you provide more information about your setup? Unless integrating the SDK manually, there shouldn't be any extra step to ensure that the SDK is able to locate the resources / localization files.

victor-sarda commented 1 year ago

Hi @lowip,

I'm answering on behalf of @Oaseru17,

At JustEat we use Tuist to generate our app project, and it doesn't use a native implementation of SPM. It handles resources and creates bundles automatically. I see you've hardcoded bundle names for SPM here.

The issue we have here is that Tuist uses a slightly different name for resource bundle it creates, for instance, with BrazeUI it will use braze_swift_sdk_BrazeUI (instead of the braze-swift-sdk_BrazeUI).

The main problem is that hardcoded bundle names for SPM aren't reliable. We had a similar issue when we migrated our modularised code base from CocoaPods to SPM, we wanted to keep support for both so we found a workaround. We created Bundle extensions like so:

extension Bundle {
    var myModuleName: Bundle {
        #if SWIFT_PACKAGE
            return Bundle.module
        #else
            // return hard coded name for Cocoapods
       #endif
}

The SWIFT_PACKAGE pre-processor macro is available by default when you build from a Package so it's safe to use. I think an update like this in the BrazeUI Resources.swift file would fix the issue:

extension Bundle {
    var resourceBundle: Bundle? {
#if SWIFT_PACKAGE
        return .module
#else
        let bundleNames = [
            // Cocoapods resources
            "BrazeUI",
        ]

        let candidates = [
            // Bundle should be present here when the package is linked into an App.
            Bundle.main.resourceURL,
            // Bundle should be present here when the package is linked into a framework.
            Bundle(for: BundleFinder.self).resourceURL,
            // For command-line tools.
            Bundle.main.bundleURL,
        ]

        for bundleName in bundleNames {
            for candidate in candidates {
                let bundlePath = candidate?.appendingPathComponent(bundleName + ".bundle")
                if let bundle = bundlePath.flatMap(Bundle.init(url:)) {
                    return bundle
                }
            }
        }

        print(
            "[braze] Error: Unable to find UI resources bundle, cannot load localizations and acknowledgments"
        )
        return nil
#endif
    }
}

@lowip please let me know if that make sense 🙏

lowip commented 1 year ago

Hi @victor-sarda,

Thanks for the context, your explanation makes sense. Unfortunately, we can't rely on the SWIFT_PACKAGE preprocessor value to guarantee the existence of Bundle.module. This is due to the fact that we also support binary XCFrameworks for BrazeUI and cannot make use of the generated Bundle.module when compiling an SPM library to a binary.

Nonetheless, I think we can provide an alternative API that should unblock you. Something like:

// Root level of BrazeUI

/// The override bundle BrazeUI uses to load resources (default: `nil`).
public var overrideResourceBundle: Bundle?

This way, you could override the bundle with the one generated by Tuist:

// In `AppDelegate.didFinishLaunching:...`

BrazeUI.overrideResourceBundle = Bundle(url: /* tuist-generated bundle url */)

If that works for you, we should be able to ship an update fairly soon.

victor-sarda commented 1 year ago

Hey @lowip,

Thanks for replying quickly. I understand the limitation with SWIFT_PACKAGE & binary xcframeworks. The proposed solution could work in theory and I'd be happy to try it 👍

Would you be able to push the changes to a new branch so we can try it on our side?

Thanks!

lowip commented 1 year ago

Hey @victor-sarda, @Oaseru17

You should be able to test the change using the branch brazeui-override-resources-bundles. If this is working for you, this change will be added to the main branch as part of the next release.

With that branch, you can now write:

BrazeUI.overrideResourceBundle = Bundle(url: /* tuist-generated bundle url */)
oriolpregona commented 1 year ago

Hi @lowip, we are facing the exact same situation in our company (Braze 5.x.x migration + Tuist). Very nice timing for resolving this issue :) I tried the suggested fix on the branch and this is the result:

Even though we did set that global Bundle before the init of Braze, the message still shows.

Screenshot 2023-03-03 at 12 40 38

It's confusing because that breakpoint is never reaching the print statement.

Screenshot 2023-03-03 at 12 41 27

Worth noting, this console error is reported right after I call Braze(configuration:...), but the debugger won't stop there at that point, it will only stop a seconds later when Braze tries to access the resources again. This might be a debugger issue, but this still doesn't explain why that error appears. Is your Braze init reaching for resources? Could that print statement come from another place too?

Also, the error was displaying twice for us, second time also complaining about a few fonts not being found. This is gone now, so I imagine that even though the error is printed, the bundle actually works.

lowip commented 1 year ago

Hi @oriolpregona,

Thanks for testing the change. The error message you are currently seeing refers to the resources bundle used by our main BrazeKit module. As the change seems to work for BrazeUI (as shown in the second snapshot), we're replicating it for BrazeKit.

Unfortunately, BrazeKit being distributed as a binary framework, we cannot provide a branch to test that behavior (to keep our repository small, our binary XCFrameworks are provided as Github releases artifacts).

We'll update here once the fix is publicly available.

oriolpregona commented 1 year ago

Hi @lowip,

Thanks for the update. I've got a few follow up questions :)

Do you have a date in mind for that update, for us to plan the migration? And what can we expect if we migrate without this fix? At the moment we use push notifications, logging and in-app messages. Would it break any of these? So far when testing, we didn't identify any issues related with the missing bundle.

In terms of the API changes, I imagine having this global variable is not the best approach you have in mind, perhaps being an option inside Braze.Configuration?

Thanks very much!

lowip commented 1 year ago

Hi @oriolpregona,

The change is merged in our private repo and will be part of the next release. We're planning to push it out later this week.

And what can we expect if we migrate without this fix? At the moment we use push notifications, logging and in-app messages. Would it break any of these? So far when testing, we didn't identify any issues related with the missing bundle.

In terms of the API changes, I imagine having this global variable is not the best approach you have in mind, perhaps being an option inside Braze.Configuration?

There are currently a few reasons explaining why we'd rather keep it global:

For the time being, we'd rather keep similar APIs for both BrazeKit and BrazeUI:

BrazeKit.overrideResourceBundle = /* ... */
BrazeUI.overrideResourceBundle = /* ... */
victor-sarda commented 1 year ago

Hi @lowip,

Sorry for the delay. Thanks for pushing a fix, I'll try it asap and get back to you!

hokstuff commented 1 year ago

Hi @Oaseru17 @victor-sarda,

We have released the Swift SDK version 5.11.1 which adds functionality that addresses this issue.

Thanks!

victor-sarda commented 1 year ago

Hi @hokstuff, thanks for letting us know, we actually planned to test it today 🎉

victor-sarda commented 1 year ago

Hi @hokstuff, the fix seems to work for BrazeUI but the issue still happens with BrazeKit BrazeKit.overrideResourceBundle doesn't seem to have any effect.

hokstuff commented 1 year ago

Hi @victor-sarda,

Can you send any relevant logs and errors you are seeing when reproducing this issue? Also, can you provide additional context or code snippets around how you are using this feature?

If it's easier to bundle all the info in an email, feel free to contact support@braze.com and link to this issue.

Thanks!

victor-sarda commented 1 year ago

Apologies, it's working when setting the bundle before initialising Braze!

oriolpregona commented 1 year ago

Thanks very much, seems to work fine on our end :)