Update domain-task package to version 2.0.1 (major bump because break…

…ing change) and modify 'fetch' behaviour so it no longer tries to register the task with domain-task automatically. See code comments for reasons.

src/Microsoft.AspNetCore.SpaServices/npm/aspnet-prerendering/package.json

@@ -1,6 +1,6 @@
 {
   "name": "aspnet-prerendering",
-  "version": "1.0.2",
+  "version": "1.0.4",
   "description": "Helpers for server-side rendering of JavaScript applications in ASP.NET Core projects. Works in conjunction with the Microsoft.AspNetCore.SpaServices NuGet package.",
   "main": "index.js",
   "scripts": {
@@ -10,7 +10,7 @@
   "author": "Microsoft",
   "license": "Apache-2.0",
   "dependencies": {
-    "domain-task": "^1.0.1",
+    "domain-task": "^2.0.1",
     "es6-promise": "^3.1.2"
   },
   "devDependencies": {

src/Microsoft.AspNetCore.SpaServices/npm/domain-task/package.json

@@ -1,8 +1,8 @@
 {
   "name": "domain-task",
-  "version": "1.0.1",
+  "version": "2.0.1",
   "description": "Tracks outstanding operations for a logical thread of execution",
-  "main": "main.js",
+  "main": "index.js",
   "scripts": {
     "prepublish": "tsd update && tsc && echo 'Finished building NPM package \"domain-task\"'",
     "test": "echo \"Error: no test specified\" && exit 1"

src/Microsoft.AspNetCore.SpaServices/npm/domain-task/src/fetch.ts

@@ -1,7 +1,6 @@
 import * as url from 'url';
 import * as domain from 'domain';
 import * as domainContext from 'domain-context';
-import { addTask } from './main';
 const isomorphicFetch = require('isomorphic-fetch');
 const isBrowser: boolean = (new Function('try { return this === window; } catch (e) { return false; }'))();
 
@@ -42,9 +41,33 @@ function issueRequest(baseUrl: string, req: string | Request, init?: RequestInit
 }
 
 export function fetch(url: string | Request, init?: RequestInit): Promise<any> {
-    const promise = issueRequest(baseUrl(), url, init);
-    addTask(promise);
-    return promise;
+    // As of domain-task 2.0.0, we no longer auto-add the 'fetch' promise to the current domain task list.
+    // This is because it's misleading to do so, and can result in race-condition bugs, e.g.,
+    // https://github.com/aspnet/JavaScriptServices/issues/166
+    //
+    // Consider this usage:
+    // 
+    // import { fetch } from 'domain-task/fetch';
+    // fetch(something).then(callback1).then(callback2) ...etc... .then(data => updateCriticalAppState);
+    //
+    // If we auto-add the very first 'fetch' promise to the domain task list, then the domain task completion
+    // callback might fire at any point among all the chained callbacks. If there are enough chained callbacks,
+    // it's likely to occur before the final 'updateCriticalAppState' one. Previously we thought it was enough
+    // for domain-task to use setTimeout(..., 0) so that its action occurred after all synchronously-scheduled
+    // chained promise callbacks, but this turns out not to be the case. Current versions of Node will run
+    // setTimeout-scheduled callbacks *before* setImmediate ones, if their timeout has elapsed. So even if you
+    // use setTimeout(..., 10), then this callback will run before setImmediate(...) if there were 10ms or more
+    // of CPU-blocking activity. In other words, a race condition.
+    //
+    // The correct design is for the final chained promise to be the thing added to the domain task list, but
+    // this can only be done by the developer and not baked into the 'fetch' API. The developer needs to write
+    // something like:
+    //
+    // var myTask = fetch(something).then(callback1).then(callback2) ...etc... .then(data => updateCriticalAppState);
+    // addDomainTask(myTask);
+    //
+    // ... so that the domain-tasks-completed callback never fires until after 'updateCriticalAppState'.
+    return issueRequest(baseUrl(), url, init);
 }
 
 export function baseUrl(url?: string): string {

src/Microsoft.AspNetCore.SpaServices/npm/domain-task/src/index.ts

@@ -0,0 +1,3 @@
+// This file determines the top-level package exports
+export { addTask, run } from './main';
+export { fetch } from './fetch';