fix: make clear that error is configuration issue not server error

[hindessm]

Since we already return a ConfigurationError on line 449 it should be reasonable to return one here rather than returning an error that suggests that the server has rejected the message.

Fixes #2137

[ae-govau]

FWIW this change broke some of our running code. We had code that would:

if err == sarama.ErrMessageSizeTooLarge {
    // log a message, bump a metric, but don't retry
}

ie for our use-case, we want to ignore messages related to a message being too large. It's OK for us to throw these out. We used to check a size before sending into Sarama, but then we had some cases where our payload was under the limit, but once headers etc were added, it was over, so we switched to checking this error code.

Noting that #543 reports that previous error code has been returned for at least 8 years, this seems a reasonably significant change... (and a good example of Hyrum's Law )

It's now not clear to me how I should check for size related issues.

[ae-govau]

Interestingly the last PR which touched that line of code was 4 years ago in 2019 where #1262 reverts a change a few days earlier (#1218) that renamed a bunch of error variables because:

renaming ErrXxx constants is an API changing change #1261

(that change is arguably less impactful than this one because it could be detected/corrected at compile time whereas we only caught this at runtime)

[puellanivis]

This change also caused a critical at my company. We were originally skipping messages on the ErrMessageSizeTooLarge as well. When combining this breaking change of functionality with a different-language client having a different and larger default message.max.bytes value, it left us with a message consumed that we could not then republish.

As mentioned the new error is also extremely error prone to check for. We’ve had to resort to:

var confErr sarama.ConfigurationError

if errors.As(err, &confErr) && strings.HasPrefix("Attempt to produce message larger than configured Producer.MaxMessageBytes", string(confErr)) {
	// handle skipping the message here
}

This code is inherently fragile, and could break if the error text changes in any way.

With this sort of behavior change, it really should have involved a breaking change removing the ErrMessageSizeTooLarge symbol, which is now unused in the package itself. At least then, users would have at least been aware of the breaking change of behavior, and not left with a landmine to eventually step on? I know that’s also considered bad-form in Golang, but it might have just been the least bad option in this case.