Version compatibility guarantees

[kubukoz]

Hi! I've tried to upgrade my direct dependency from 1.27.2 to 1.28.0, but I hit a java.lang.NoSuchMethodError: 'software.amazon.smithy.model.node.ObjectNode software.amazon.smithy.model.traits.ExamplesTrait$Example.getOutput()'.

A bigger stack trace

java.lang.NoSuchMethodError: 'software.amazon.smithy.model.node.ObjectNode software.amazon.smithy.model.traits.ExamplesTrait$Example.getOutput()'
    at software.amazon.smithy.openapi.fromsmithy.protocols.ExampleNode$.$anonfun$forOutputMembers$1(ExampleNode.scala:69)
    at scala.collection.immutable.List.flatMap(List.scala:293)
    at software.amazon.smithy.openapi.fromsmithy.protocols.ExampleNode$.forOutputMembers(ExampleNode.scala:68)
    at software.amazon.smithy.openapi.fromsmithy.protocols.AlloyAbstractRestProtocol.$anonfun$createResponseDocumentIfNeeded$2(AlloyAbstractRestProtocol.scala:616)
    at software.amazon.smithy.openapi.fromsmithy.protocols.AlloyAbstractRestProtocol.$anonfun$createExamples$5(AlloyAbstractRestProtocol.scala:672)
    at scala.collection.immutable.List.map(List.scala:246)
    at software.amazon.smithy.openapi.fromsmithy.protocols.AlloyAbstractRestProtocol.$anonfun$createExamples$4(AlloyAbstractRestProtocol.scala:672)

I checked with mima-cli (a CLI for MiMa, the binary compatibility checking tool mostly used for Scala, but based on Java Classfiles), and it gave me this:

software.amazon.smithy.model.traits.ExamplesTrait#Example.getOutput: IncompatibleResultType

which would make sense, as apparently getOutput's return type was wrapped in Optional: https://github.com/awslabs/smithy/pull/1599/files#diff-7015682a8fef7e05895ae90e58bc42581549813443c572b60aabea9031323c78L136

If this library makes no guarantees on binary compatibility between minor versions, perhaps that should be covered in the README?

[kstich]

Given the updates to the @examples trait in #1599, the old trait behavior was broken. Our options for fixing this in code were either:

1. return an empty ObjectNode, incorrectly rendering examples with both an output and an error,
2. make a new method and deprecate the old one, leaving the old one possibly generating NullPointerExceptions, or
3. update it to use Optional, technically breaking binary compatibility.

We potentially should have gone the first or second routes, but leaned towards being consistent with our codebase's use of Optional when fixing the bug described in #1582. I'd hesitate to add a note to the README, as this should only occur when fixing a related bug.

[kubukoz]

Sure, I understand the old behavior wasn't correct, but if a user has an older version of another artifact (e.g. the openapi module), they may still be transitively using the method. Updating part of the stack will result in NoSuchMethodError.

What I'm suggesting is that the README clarify (in the general case) that there may be binary incompatibilities between artifacts with the same major version.

[kubukoz]

The specific case that triggered this for me: having dependencies on

• com.disneystreaming.smithy:smithytranslate-transitive_2.13:0.3.2 - this depends on smithy-model 1.28.0
• com.disneystreaming.alloy:alloy-openapi_2.13:0.1.11 - this depends on smithy-openapi 1.27.2 (and, in turn, smithy-model 1.27.2)

The build tool used 1.28.0 for smithy-model and 1.27.2 for smithy-openapi, leading to the unfortunate missing method. Ha I known binary breakage is allowed, I would've made the build tool more constrained in how it resolves version conflicts 😅 which is why I'm asking about the general promises these libraries make - so that my tooling can be adjusted.

[Baccata]

This is indeed a reasonable concern : the expectation from downstream users of libraries that use the semver versioning pattern is that minor versions are binary backward compatible. Introducing binary incompatible changes should be communicated to downstream users via bumps of major versions.

If binary-compatibility is something that AWS is not willing to guarantee between minor versions, it'd certainly be nice to have a warning somewhere in the documentation, so that downstream library authors can take steps to protect their users accordingly.

[mtdowling]

Sorry about the break. We should have come up with a different way to address the issue. We won't add a note to the README at this time because it shouldn't happen again, and we will make every reasonable attempt to maintain binary compatibility (i.e., breaking the return method signature is not that).

[kubukoz]

@mtdowling thanks!