define semantics of {source,sink}.finally()

[isonmad]

Discussion at #617 (comment) #617 (comment) #620 (comment) and #628 (comment) mentioned the need for adding a finally() method to underlyingSink (and underlyingSource?) at some point in the future.

It sounds like the ideas for finally are

• called after abort, close, (and cancel and flush?) etc, are called (after their returned promises fulfill?)
• called with any uncaught errors thrown synchronously by any method, or thrown by strategySize, or thrown by the implementation when e.g. strategy.size() returns NaN
• also called with the rejection reason of any rejected promise that causes the stream to become errored

Proposed arguments were finally(why, reason), where why is e.g. the string 'close' or 'abort', but what about all the other causes for finally to be called? What would 'why' be for an invalid queueing strategy return value causing a RangeError?

Should it stay as two arguments like that, or one object analogous to an IteratorResult?

[ricea]

Thanks for filing this.

My plan for why was to add a few more reasons, such as 'error' for when controller.error() was called. I intended to use 'exception' for when a sink/source method threw an exception, but I realise now that we need to distinguish exceptions from strategy.size().

Could you explain a bit more what the benefit would be of switching to one argument?

[ricea]

I think sometimes we will need to call finally() asynchronously because we are in a bad state and can't deal with re-entrancy issues. It might be good to guarantee that we always call finally() asynchronously, so that sink code can be sure that none of its own code is executing synchronously. On the other hand, it may be sufficient to guarantee that none of the sink methods are on the stack.

Everything applies the same to ReadableStream and TransformStream. WritableStream is just what's on my mind right now.

[jedwards1211]

when a sink/source method threw an exception

Do thrown errors/rejections from the underlying source/sink get wrapped in a DOMException elsewhere? Because usually what was thrown or the rejection reason is an "error"...

[jedwards1211]

For ReadableStreams maybe why should include 'startThrew', 'startRejected', 'pullThrew', etc

[ricea]

These days I'm leaning towards providing no parameters to finally(), to match the way it works in JavaScript and Promise.prototype.finally.

It means that if your stream owns a resource that can only be disposed of once, you'll have to track that yourself in your source/sink implementation, but I suspect people would prefer to do that for robustness anyway.

I mean something like

const source = {
  start() {
    this._need_dispose = true;
    this._resource = AcquireResource();
  },
  cancel() {
    DisposeResource(this._resource);
    this._need_dispose = false;
  },
  finally() {
    if (this._need_dispose) {
      DisposeResource(this._resource);
    }
  }

This is not actually a good example because you could just always leave the disposal until finally() is called.

Anyway, if finally() takes no arguments we can spec and implement it quickly* and finally close this hole in the spec.

*Subject to writing many, many, many wpts.