[Guide] creating a promise and continue to run steps async

[marcoscaceres]

Usually, when we write async stuff in specs we do something like this:

The steps to do a foopy are given by the following algorithm. The algorithm runs asynchronously and returns a Promise:

1. Let p be a newly-created promise.
2. Return p and run the remaining steps asynchronously.
3. If the foopy is not bary enough, then:
   • 3.1. Let e be a new Error...
   • 3.2. Reject p with e
4. Resolve p with a new Baz

In the examples given in the spec writing guide, step 2 is omitted. Is this on purpose? Or is doing something like 2. wrong and specs should be written differently?

[domenic]

I have always found that phrase confusing and bizarre, and would hope it could be replaced with something either implicit (like in the examples I give, e.g. "in x seconds, resolve p with bar"), or more like:

1. Let p be a newly-created promise.
2. Queue a microtask to perform the following:
   1. If the foopy is not bary enough, then reject p with a new Error indicating the lack of bary-ness.
   2. Otherwise, resolve p with a new Baz.
3. Return p.

But this example is too contrived; why would we queue a microtask to do this? Usually we need to wait on some external input. We should perhaps examine existing examples.

That said, this isn't really a battle worth fighting, probably; if people really like "return x but then magically somehow also do things after the return statement has executed" then I guess it's somewhat clear what that means, namely, that the following steps should actually be transposed before the return statement, but in some kind of asynchronous execution block.

[marcoscaceres]

The problem is that we don' have "microtasks" defined anywhere in the platform yet. In the mind of the spec writer, they are just following the assumed execution flow of ES.

If you want a real example, I'm just in the middle of rewriting the following using your guide: http://manifest.sysapps.org/#dfn-steps-to-request-to-add-bookmark

(note that the url argument has been dropped in my rewrite, but that gives you an idea)

[marcoscaceres]

Probably a bit early to use the example I gave, actually. It needs a big rewrite.

[domenic]

In the mind of the spec writer, they are just following the assumed execution flow of ES.

But how does this make sense? In ES, you can't perform any steps at all after you return. That's why it's so confusing.

[marcoscaceres]

Right, the unspoken assumption is that spec text is running in the WebIDL limbo... the ghastly place between ES and native code and where the dammed await judgement :)

[marcoscaceres]

How about:

The steps to request to add a bookmark are given by the following algorithm. The algorithm returns a promise.

1. Let promise be a newly-created promise.
2. [Queue a task](http://www.whatwg.org/specs/web-apps/current-work/#task-queue) to perform the following:
   1. If this method was not invoked as a result of explicit user action, then:
      1. Let error be a new DOMException whose name is "SecurityError".
      2. Reject promise with error.
   2. If the document's mode of operation is standalone:
      1. Let error be a new DOMException whose name is "NotSupported".
      2. Reject promise with error.
   3. Let info be the result of getting a web application's metadata.
   4. Using info, and in a manner that is user-agent specific, allow the end user to make a choice as to whether they want to add the bookmark. If the end-user aborts the request to add the bookmark (e.g., they hit escape, or press a "cancel" button):
      1. Let error be a new DOMException whose name is "AbortError".
      2. Reject promise with error.
   5. Resolve promise with undefined.
3. Return promise.

[domenic]

That looks lovely. Minor points:

• as discussed, DOMException should not be used with promises. (Or it has to be fixed before promise-based APIs are deployed. Stack trace-less "exception"s suck!) best thing to do for now might be to use Error and file an issue to switch once DOM gets fixed.
• you probably want to return after you reject instead of continuing onward in the algorithm.
• I don't really see a need to create error objects first then on a separate line reject; seems like it could be just as precise in one line, but more concise.
• no real need to queue a task for those steps, I don't think. If writing that stuff in JS you wouldn't put it inside setImmediate or similar.

[marcoscaceres]

as discussed, DOMException should not be used with promises. (Or it has to be fixed before promise-based APIs are deployed. Stack trace-less "exception"s suck!) best thing to do for now might be to use Error and file an issue to switch once DOM gets fixed.

I know you are right in that I should do that - but I don't want to do the work twice. I'll add a big red note in my spec and link to the bug noting that DOMException has to extend ES Error has to be fixed before the spec can be implemented. The spec is a long way away from being implementable - so right now it's just for show and tell.

I hope that's ok.

you probably want to return after you reject instead of continuing onward in the algorithm.

Ok, I'll do, for example "Reject promise with a new DOMException whose name is "AbortError" and return."

I don't really see a need to create error objects first then on a separate line reject; seems like it could be just as precise in one line, but more concise.

Agree. As above.

no real need to queue a task for those steps, I don't think. If writing that stuff in JS you wouldn't put it inside setImmediate or similar.

The problem is that ... there is no setImmidiate() yet across browsers and the other way to do it is using a bunch of hacks :cry:. I hear ES7 will fix this ... or will get some magic way of getting access to the task queues (you probably know more about this, and I'm pretty sure we've discussed this before). Anyway, in W3C spec land, the closest we have to those is the concept of "queue a task". I'm going to keep that for now as it's not particularly harmful.

[domenic]

I'll add a big red note in my spec and link to the bug noting that DOMException has to extend ES Error has to be fixed before the spec can be implemented.

Seems perfect :)

The problem is that ... there is no setImmidiate() yet across browsers and the other way to do it is using a bunch of hacks

No, you misunderstood me. Think of how you would implement the above algorithm in JS. It would not be:

function algorithm() {
  return new Promise(function resolver() {
    // Note: `resolver` is not called in the next tick; it is called immediately.

    // Now "queue a task":
    setImmediate(function () {
      if (methodWasNotInvokedAsAResultOfExplicitUserAction()) {
        reject(new DOMException("bad stuff", { name: "SecurityError" }));
        return;
      }
      if (documentsModeOfOperationIsStandalone()) {
        reject(new DOMException("bad stuff", { name: "NotSupported" }));
        return;
      }
      var info = getWebApplicationsMetadata();

      userAgentSpecificChoooser(function (result) {
        if (!result) {
          reject(new DOMException("bad stuff", { name: "AbortError" }));
          return;
        }
        resolve(undefined);
      });
    });
  });
}

Instead you would just do:

function algorithm() {
  return new Promise(function resolver() {
    // Note: `resolver` is not called in the next tick; it is called immediately.

    if (methodWasNotInvokedAsAResultOfExplicitUserAction()) {
      reject(new DOMException("bad stuff", { name: "SecurityError" }));
      return;
    }
    if (documentsModeOfOperationIsStandalone()) {
      reject(new DOMException("bad stuff", { name: "NotSupported" }));
      return;
    }
    var info = getWebApplicationsMetadata();

    userAgentSpecificChoooser(function (result) {
      if (!result) {
        reject(new DOMException("bad stuff", { name: "AbortError" }));
        return;
      }
      resolve(undefined);
    });
  });
}

There is no need to artificially queue a task.

[marcoscaceres]

Ok, got ya! I had a feeling that's what you meant. I was not sure about it being in the next tick or not. I think the above is something important to put into the guide. We've gotten really used to waiting for the next tick (unnecessarily) with "queue at task" and I think people are going to continue to fall into that trap with promises (given that they are kinda new and not well understood).

[domenic]

Awesome, will be sure to do so!

[domenic]

@marcoscaceres I added some guidance under Maintain a Normal Control Flow and Do Not Queue Needless Tasks. I also created an updated version of your addBookmark function.

I wrote the section Rejections Should Be Used For Exceptional Situations with addBookmark in mind. Is the user denying permission really exceptional? Maybe, maybe not, but let me know what you think.

[cscott]

So I just read through the guidance section. I think your comment https://github.com/domenic/promises-unwrapping/issues/85#issuecomment-29958070 is actually clearer than what the guidance says right now. The then action of the Promise is still going to get invoked asynchronously, with other queued tasks possibly intervening. What your code example above makes clear is that the "validate arguments" and "getWebApplicationsMetadata" steps occur synchronously -- that is, there is no race if some other queued task alters the arguments or changes the "web applications metadata" asynchronously; the promise is already resolved.

So it would be nice to clarify the guidance further. Perhaps "Do Not Queue Needless Tasks" can explicitly mention that argument validation should be synchronous, even though the user experiences the promise rejection asynchronously. Also/alternatively, the "addBookmark" example might explicitly state, "Note that argument validation and fetching the web application's metadata is done synchronously, even though notification is asynchronous. This prevents races with another task which might mutate the arguments or application metadata."

[domenic]

Thanks for the feedback @cscott. The guide has largely moved to w3ctag/promises-guide, and we are discussing similar issues at https://github.com/w3ctag/promises-guide/issues/7