Tracking Issue for setOnCloseHandler being Experimental

[morgwai]

To resolve issue #5895 PR #8452 has been created that adds new ServerCallStreamObserver.setOnFinishHandler(...) method. The handler is called by Listener.onComplete when the call is finished correctly from the server's point of view: either onCompleted() or onError(Throwable) has been called, all the messages and trailing metadata have been put on the wire and the stream has been closed.

Several names were proposed for the handler:

• onCompleteHandler : derives name from Listener's method but causes confusion with StreamObserver.onCompleted()
• onSuccessHandler : my initial idea, yet also confusing as it can be called also after StreamObserver.onError(...)
• onFinishHandler : current approach, matches well the verb from method's javadoc
• onFinalizeHandler : would also probably do well

[ejona86]

API review notes:

• onCompleteHandler
• onCompleteOrErrorFinishedHandler
• onSuccessHandler
  • -2
• onFinalizeHandler
  • Would be fairly apt if it got called on cancellation as well
  • -1 (strong -1)
  • -1
  • Ironic +1 (since "finalize" is such a dirty word in Java)
• onCompleteOrErrorHandler
  • Confusing relationship with onCancel?
• onFinishHandler
  • +1
  • -1
• onDoneHandler
  • +1
  • -1
• onNonCancelledStatusHandler
• onCallCompleteHandler/onCallCompletionHandler
  • +1
• onCloseHandler/onClosedHandler
  • This is to mirror terminology in the ClientCall.Listener/ServerCall. There was some split feelings earlier about making new terminology vs clarifying existing terminology
  • onClosed is a mismatch with “onCancelHandler” (it isn’t onCancelledHandler)
  • onClose is easier to type
  • We use “close” many other places in the API (e.g., client-side)
  • onClosed makes it more clear that the operation is complete, instead of just initiated
  • onClose
    • +3
  • onClosed
    • +3
  • No strong opinions on either side. Decided onClose, “just because”

So that voting isn't our normal voting just at the end. We put votes in as we discussed and it was more +1/-1 in a ACK/NACK sense. We did a typical vote (one vote per person) for onClose/onClosed since that was the clear set of favorites, beating out all the other options except tying with onCallCompleteHandler for one person.

It was actually a bit strange how long it took for onCloseHandler to be put on the table. But discussion quickly centered around it after it was proposed.

@morgwai, let's change the PR to setOnCloseHandler().

[morgwai]

I'm on it :)

just 1 cent regarding onClose vs onClosed: since onCancel is exactly onCancel and not onCancelled, it's better to keep tenses consistent.

[ejona86]

since onCancel is exactly onCancel and not onCancelled, it's better to keep tenses consistent.

Yeah, that was one of the arguments ("onClosed is a mismatch with “onCancelHandler” (it isn’t onCancelledHandler)"). However, onCancel and onCancelled have the same meaning since cancellation is essentially instantaneous. The problem with close is it takes a while and so the two names could be viewed to have different meanings ("onClosed makes it more clear that the operation is complete, instead of just initiated").