search

akka / akka-http

The Streaming-first HTTP server/module of Akka
https://doc.akka.io/docs/akka-http
Other
1.34k stars 594 forks source link

Consider returning `null` instead of `Done` for `ServerBinding.unbind` #2177

Open jrudolph opened 6 years ago

jrudolph commented 6 years ago

I knew that https://github.com/akka/akka-http/pull/1908 was binary incompatible but I didn't predict that

  1. it would be used in library code (it was, see https://github.com/akka/akka-management/pull/258/files/41dcb046448c8d80cd791f13ff9f4c85af47bf3f#r207826174)
  2. it would lead to runtime errors because .map(_ => ...) is still adding a cast to unmentioned parameter type

We could either just leave it as is and hope for the incompatible universe to upgrade or we could just return null instead of Done and hope that no one actually accesses the value.

dwijnand commented 5 years ago

Is this something that MiMa failed to identify?

raboof commented 5 years ago

Is this something that MiMa failed to identify?

Yes

dwijnand commented 5 years ago

Ah right, this is https://github.com/lightbend/migration-manager/issues/40. 😭

jrudolph commented 5 years ago

I knew that #1908 was binary incompatible

Actually, it's even arguable if this change is "binary incompatible". In akka-management, it fails with a ClassCastException but does that really put it onto a higher level than other of minor (behavioral) incompatibilities we introduce with many changes in patch releases?

dwijnand commented 5 years ago

Yeah, I'd say so, because it hard fails to link. (edit: or cast, as the case may be)