confluentinc / confluent-kafka-dotnet

Confluent's Apache Kafka .NET client
https://github.com/confluentinc/confluent-kafka-dotnet/wiki
Apache License 2.0
18 stars 852 forks source link

Unintentional Breaking Change to TopicPartitionOffset in 2.1 #2060

Closed drinehim closed 1 year ago

drinehim commented 1 year ago

Description

Version 2.1 (specifically commit 9826d794) introduced a breaking change (looks unintentional as you would think optional parameters would be non-breaking but they aren't). This is causing my applications that reference a library that uses a library that uses Confluent.Kafka to break when the entry app tries to upgrade to the latest Confluent.Kafka.

Library -> Confluent.Kafka 2.0.2 App -> Confluent.Kafka 2.1.1

Exception: RLI.EventMessaging.Client.ClientHandler[0] Method not found: 'Void Confluent.Kafka.TopicPartitionOffset..ctor(System.String, Confluent.Kafka.Partition, Confluent.Kafka.Offset)'. System.MissingMethodException: Method not found: 'Void Confluent.Kafka.TopicPartitionOffset..ctor(System.String, Confluent.Kafka.Partition, Confluent.Kafka.Offset)'. at Confluent.Kafka.Consumer2.Consume(Int32 millisecondsTimeout) at RLI.EventMessaging.Kafka.Client.KafkaClient3.<>c__DisplayClass31_01.<ConsumeWithRetryAsync>b__0() in C:\code\RLI.EventMessaging\src\RLI.EventMessaging.Kafka\Client\KafkaClient.cs:line 421 at System.Threading.Tasks.Task1.InnerInvoke()

How to reproduce

KafkaBreakTest.zip

Library -> Confluent.Kafka 2.0.2 App -> Confluent.Kafka 2.1.1

Kafka Version: Any Client Configuration: Basic OS: Linux (probably windows too) Critical: No Broker Logs: Can't get that far

Checklist

Please provide the following information:

anchitj commented 1 year ago

I tested the code you provided in the zip file and it works fine with v2.1.1. Can you test again with a clean build and confirm if this is an issue?

anchitj commented 1 year ago

Please reopen if this is still an issue.

drinehim commented 1 year ago

@anchitj I still get the issue with the zip. Yes if I update the version of Confluent.Kafka in KafkaLibrary it works, but that is not something that can always be done. If you use something like kafka-streams-dotnet you have no control of when the library gets updated.

I did update the Confluent.Kafka version in my company's library, so we are not currently broken. I wanted to leave this fix (#2061) to help others.

rising-4ce commented 1 year ago

The issue is completely reasonable: the breaking change of the public API must not happen with the increase of the minor version, according to the Semantic Versioning 2.0.0. By changing the constructor signature, you broke a lot of packages with dependencies on Confluent.Kafka v2.*

Considering that the solution to the problem is very simple and consists of literally replacing the constructor with an optional parameter with several constructors, this must be done.

rising-4ce commented 1 year ago

I think the ticket should be reopened.

vodesoft commented 1 year ago

I still get an error System.MissingMethodException Method not found: 'Void Confluent.Kafka.TopicPartitionOffset..ctor(Confluent.Kafka.TopicPartition, Confluent.Kafka.Offset)'.

The reason is an extra parameter in constructor was introduced (instead of creating a separate one with extra parameter leaderEpoch).

ttd2089 commented 1 year ago

The 2.1 release has at least one other breaking change. A method was added to the IConsumer interface which breaks outside implementations of the interface.

https://github.com/confluentinc/confluent-kafka-dotnet/pull/2027/files