Returning nil from a plugin crashes the app

[nkristek]

When implementing a plugin with type enrichment and returning nil from execute(event:), the app crashes, because the event is force unwrapped here.

Allowing nil as a return value would make it possible to conditionally filter events using a plugin.

Expected Behavior

When a plugin returns a nil value after enrichment, the event should be ignored.

Current Behavior

The event is subsequently force unwrapped, which is nil, so the app crashes.

Possible Solution

Add a conditional unwrap of the event variable to the existing if let statement which wraps the call to the mediator:

internal func applyPlugin(pluginType: PluginType, event: BaseEvent?) -> BaseEvent? {
    var result: BaseEvent? = event
    if let mediator = plugins[pluginType], event {
        result = mediator.execute(event: event)
    }
    return result
}

Steps to Reproduce

1. Implement and add this plugin:

class FilterPlugin: Plugin {
    var type: PluginType { .enrichment }

    func setup(amplitude: Amplitude) { }

    func execute(event: BaseEvent) -> BaseEvent? {
        guard event.eventType != "abc" else {
            return nil
        }
        return event
    }
}

2. Track an event with the eventType that is checked against in the guard (in this example "abc").

Environment

• SDK Version: v1.1.0
• OS Info: Any

[izaaz]

@nkristek - this isn't supported currently and I've changed this to an enhancement. Currently the plugin architecture is meant to enhance events with extra properties.

There are controls within Amplitude that let you block events on the server side. Is there a reason you prefer blocking and filtering events on the client side?

[theolampert]

It doesn't really matter if this is a supported use case or not, I think this is a bug and should be labelled as such, it's best practise not force unwrap in this situation and rather safely unwrap or not allow nil to be passed to that function at all.

[mcrollin]

As advertised, this method is supposed to accept an optional event yet results in a crash when provided nil. This inconsistency not only represents a semantic oversight but also qualifies as a bug, rather than a potential enhancement. A method designed to handle optional values should gracefully manage nil cases to ensure robustness and reliability of the SDK. It's not nitpicking rather a fundamental aspect of the language's type safety.

Please revise your qualification of this issue.

[izaaz]

Apologies and fair point folks. I agree that the interface should not allow a nullable event if it cannot process one. I've changed it back to a bug and we have a PR opened to allow event filtering.

[yuhao900914]

Hi guys. Thanks for choosing Amplitude.
The issue has been fixed. Please upgrade to the latest Amplitude Swift SDK v1.3.4 to test it out. Thank you.