promises-aplus / progress-spec

Discussion and drafts of a possible promise progress signalling spec
5 stars 1 forks source link

Background #1

Open ForbesLindesay opened 11 years ago

ForbesLindesay commented 11 years ago

Some of the promises libraries now implement progress. The key thing that has yet to be decided upon is how we should deal with exceptions thrown in progress handdlers.

It has been broadly agreed upon that progress should be a single value (akin to fulfillment and rejection). We have also agreed that the third argument to then should be treated as the progress handler and should also controll propagation of progress.

I'd apreciate it if someone could fill in what the currently in use rules are regarding progress propagation.

domenic commented 11 years ago

Progress propagation: given

var promise2 = promise1.then(undefined, undefined, onProgress1);
var promise3 = promise2.then(undefined, undefined, onProgress2);
promise3.then(undefined, undefined, onProgress3);

Other issues: does progress get replayed? What happens if you return a promise as your progress value? How should cases like this be handled?


Another piece of background: many people believe progress should not belong to promises at all. E.g. promises should just be event emitters and fire off progress events, or allow them to be dispatched. This introduces two difficulties, in my mind:

  1. It fails to separate the "notifier" role from the "listener" role. I.e. it should be the resolver's responsibility to trigger progress events, while it's the promise's responsibility to provide a hook for listening to them. Most event emitter implementations (e.g. EventEmitter in Node or EventTarget in the DOM) conflate the two.
  2. It requires manually propagating progress, negating much of the use of promises and leading to ugly code.

If neither point is addressed, it turns code like this (which currently works in Q)

function getUser() {
   return doJsonXhr("/user").then(function (res) {
      return res.user;
   });
}

into this code:

function getUser() {
  var p = doJsonXhr("/user");

  var derived = p.then(function (res) {
    return res.user;
  });

  p.on("progress", function (x) {
    p.emit("progress", x);
  });

  return derived;
}

If only point 1 is addressed, but point 2 is not, the code is much worse than even that.

ForbesLindesay commented 11 years ago

How do you stop progress propagation in your first example? You could deliberately address 2 and not 1 by having a simple method to do straight forward direct propagation within then by passing true as the third argument. You'd then let people do the more complex event emitter stuff when they needed to handle more complex stuff like fast then slow.

domenic commented 11 years ago

To stop progress propagation, add a progress handler that returns undefined.

domenic commented 11 years ago

Hmm, actually, that won't stop propagation, it'll just propagate undefined instead of anything useful. Good point.

ForbesLindesay commented 11 years ago

I'm in favour of also making undefined not propagate. I think it would be nice to have a value that succinctly says "don't propagate"

domenic commented 11 years ago

Reminds me of StopIteration from ES6. Maybe throwing a distinguished error would make more sense there?

ForbesLindesay commented 11 years ago

Yeh, throwing an exception would work, but it may be difficult to distinguish the right exception. An instanceof check would prevent using them interoperably

briancavalier commented 11 years ago

I can't see any reason not to propagate undefined, especially since it propagates like any other value for fulfill/reject. I do see the value in having a StopIteration like value, but I also see the problems around interop there since it's not something that the language can provide right now ... unless we were to actually use StopIteration, which would require a polyfill, and just feels wrong.

Throwing seems reasonable, but raises the question of what to do with the thrown value. It seems like that may be a candidate for a promise debugger hook similar to the stuff we're discussing in unhandled-rejections-spec land

ForbesLindesay commented 11 years ago

How about instead of using instanceof we add a .code property to the error with a value of 'StopProgressPropagation'. By just checking for that would allow all libraries to use other libraries' exceptions interchangeably. Then it could be upgraded to an instanceof check if it gains language support.

domenic commented 11 years ago

@ForbesLindesay or .name, to match existing errors.

ForbesLindesay commented 11 years ago

I'm not at all fussed what we call the property, I thought node.js used .code a lot elsewhere? I'm happy to use .name if people prefer.

briancavalier commented 11 years ago

afaik, .name is what builtins like TypeError use. If we go with a particular exception name, then an implementation will assume that whatever thing it catches is indeed an error, compare it's .name to the "official" name, and if it matches, silently stop progress propagation? That seems reasonable.

Would it also be reasonable to return one of these "stop propagation" objects instead of throwing it?

What about other exceptions? Should we assume that we'll be able to rely on some debug hook? Crash the app with a next-tick throw?

domenic commented 11 years ago

Let's not forget we can open more than one GitHub issue for the separate issues involved ;)

juandopazo commented 11 years ago

I haven't been exposed to that many situations involving promises as you have, so my thoughts are probably missing something.

I can think of "progress" in two terms: progress steps taken during the life of a promise and progress through a promise chain. I'll exemplify:

request(url).then(function (html) {
  // HTTP sends chunks which correspond to a progress in the HTTP request 
});

// there are three steps here that constitute progress
async1().async2().async3().then(function (result) {
});

I don't think progress in the sense of the first example has a place in a promises implementation. In the world of asynchronously calculated values we've seen three major approaches:

The interesting part is that complexity increases with the type of approach chosen. A callback only requires understanding of scopes and asynchronicity. Promises require understanding of asynchronicity and control flow using objects. Streams require understanding the previous knowledge and the granularity of events, pausing, resuming, etc.

I think that intermediate states belong to Streams and that Promises should remain boolean, with only a fulfilled or rejected state. This will be easier to communicate, teach and maintain. As you're already showing, more granular interaction with promises starts to delve in the realms of event emitters which in a way defeat the purpose of promises: a lightweight object representing the state of an asynchronous transaction. I also think more complexity will be unhelpful when trying to get native providers (DOM, WinJS, Node, etc) to adhere to a standard.

In the case of the second example, I haven't run across an implementation that captures the progress of the promise chain. Does it need any special considerations or is it doable with just then?

ForbesLindesay commented 11 years ago

I disagree. Streams have complexity mostly because of their desire to have a super low memory footprint and be really efficient. They were never meant to simplify things in the way promises are. I often use helpers which do things like: stream a file from a web-server to the local file system. That has a boolean granularity, so I want to return a promise for it for simplicity, but I might still want to display progress to the user. I'd describe that as UX (user experience) progress.

In the chaining progress option, it could be for two reasons. Their might be something I can do with the partial results, or I might simply want to display what stage of the operation I'm at.

I've not actually had occasion to use the first (UX) type of progress, but I can see it's value. It is especially valuable in client-side JavaScript where you might do a file upload (boolean granularity with UX progress). component/upload is a good example of a library that uses this kind of progress (it's not a real promise, but it's trying to be a promise).

I do have an occasion to use the second type of progress, where I am able to do stuff with partial results. It definitely needs special consideration, because I have one method which makes total sense and does a series of about 20 things one after another. It only ever makes sense to call in that way, but sometimes I'll want to display the intermediary results, so I need progress.

juandopazo commented 11 years ago

Should I understand the lack of other answers besides @ForbesLindesay's as consensus?

Personally I have to say I'm not very interested. Promises as defined in A+ offer a lot of value with a very small code footprint. That allows us to incorporate it into libraries like jQuery or YUI without increasing the KB weight by as much as a fully featured event system. Also, like I said before, the smaller it is, the easier it is to teach, learn and evangelize.

domenic commented 11 years ago

I am somewhere in between. Ideally I think a promise for a stream is the way to go, e.g.

sendRequest.then(function (response) {
   response.on("progress", function () {});
   response.readToEnd().then(function (allData) {
   });
});

I agree that progress is not that compelling, but, on the other hand, people do use it in the real world---mostly for UI programming on the client side. And having an interoperable way to deal with it seems prudent, given its presence in most implementations these days.

sebarina commented 7 years ago

does promisekit support progress now?