Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promesa should not provide print-method impl for CompletionStage #151

Open
DerGuteMoritz opened this issue Jan 22, 2024 · 1 comment
Open

Comments

@DerGuteMoritz
Copy link

Promesa provides an implementation of clojure.core/print-method for java.util.concurrent.CompletionStage. This can lead to surprising ambiguity errors in application code (see e.g. clj-commons/manifold#237) which then must be worked around via prefer-method. Since neither the type nor the multimethod are owned by Promesa, the error doesn't give any indication as to where this ambiguity stems from which makes it tricky to track down.

Additional rationale: Since print-method only dispatches on the type of the first argument, the Guidelines for extension of protocols arguably apply here, as well. The first of these cover this particular case:

If you don’t own the protocol or the target type, you should only extend in app (not public lib) code, and expect to maybe be broken by either owner.

@ferdinand-beyer
Copy link

Adding to this, not sure if this should be a separate bug report:

Promesa's print-method for CompletionStage uses the IState protocol for (pt/-pending? x) etc. However, CompletionStage does not satisfy this protocol, CompletableFuture does. As a result, printing a CompletionStage that is not a CompletableFuture will throw exceptions.

In general, Promesa's JVM implementation assumes in some places that a promise is a CompletableFuture, e.g. p/all and p/race are implemented using CompletableFuture/allOf and CompletableFuture/anyOf, expecting CompletableFuture arguments but not CompletionStage. The protocols seem to suggest that one can provide custom types that satisfy the protocols, but this will fail in seemingly random places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants