Report asynchronous module loader errors
While implementing this, I've found one memory leak error in the JSStdFunction... I'll create the separate patch to fix that.
Created attachment 259587 [details] Patch
Comment on attachment 259587 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259587&action=review The patch is ready. Added comment for the ease of the review. > Source/JavaScriptCore/jsc.cpp:1375 > + }); The error handler function to catch the asynchronous errors and report them. > Source/JavaScriptCore/jsc.cpp:1388 > + promise = evaluateModule(globalObject->globalExec(), fileName); Load the module with the entry point moduleName. > Source/JavaScriptCore/jsc.cpp:1407 > + promise = evaluateModule(globalObject->globalExec(), jscSource(script, fileName)); Load the module with the source string. > Source/JavaScriptCore/jsc.cpp:1409 > + promise->then(globalObject->globalExec(), nullptr, errorHandler); JSInernalPromise.then with the error handler to report the asynchronous errors. > Source/JavaScriptCore/jsc.cpp:1410 > globalObject->vm().drainMicrotasks(); This draining ensures that the loading, execution and error reporting of the module are done here before loading the next file. > Source/JavaScriptCore/runtime/JSInternalPromise.h:42 > +// CAUTION: Must not leak the JSInternalPromise to the user space to keep its integrity. Add the note about JSInternalPromise. > Source/JavaScriptCore/runtime/JSInternalPromise.h:52 > + JS_EXPORT_PRIVATE JSInternalPromise* then(ExecState*, JSFunction* = nullptr, JSFunction* = nullptr); We just add "then" helper only to JSInternalPromise (we don't add it to JSPromise) because "then" method may be replaced/modified in the user-exposed Promises. > Source/JavaScriptCore/runtime/ModuleLoaderObject.cpp:150 > + return jsCast<JSInternalPromise*>(call(exec, function, callType, callData, this, arguments)); Since "requestInstantiateAll" is builtin internal method, we can ensure that the result of this function is always JSInternalPromise*. > Source/JavaScriptCore/runtime/ModuleLoaderObject.cpp:164 > + return jsCast<JSInternalPromise*>(call(exec, function, callType, callData, this, arguments)); Ditto. > Source/JavaScriptCore/runtime/ModuleLoaderObject.h:74 > + JSInternalPromise* instantiate(ExecState*, JSValue key, JSValue source); Ensure the result value is JSInternalPromise* by type!
Created attachment 259588 [details] Patch add ifdef guard for promise = ...
The patch is ready. The following is the loading results. ===== tmp/main.js import "tmp/test001.js" ===== tmp/test001.js import 'tmp/arith.js' import 'tmp/test001.js' ===== tmp/arith.js import "tmp/test001.js" export function add(a, b) { return a + b; } = result $ Tools/Scripts/run-jsc -m tmp/main.js Running 1 time(s): DYLD_FRAMEWORK_PATH=/Users/yusukesuzuki/development/Modules/WebKitBuild/async/Release /Users/yusukesuzuki/development/Modules/WebKitBuild/async/Release/jsc -m tmp/main.js Loader [resolve] tmp/main.js Loader [fetch] tmp/main.js Loader [translate] tmp/main.js Loader [instantiate] tmp/main.js Analyzing ModuleRecord key('tmp/main.js') Dependencies: 1 modules module('tmp/test001.js') Import: 0 entries Export: 0 entries Loader [resolve] tmp/test001.js Loader [fetch] tmp/test001.js Loader [translate] tmp/test001.js Loader [instantiate] tmp/test001.js Analyzing ModuleRecord key('tmp/test001.js') Dependencies: 2 modules module('tmp/arith.js') module('tmp/test001.js') Import: 0 entries Export: 0 entries Loader [resolve] tmp/arith.js Loader [resolve] tmp/test001.js Loader [fetch] tmp/arith.js Loader [translate] tmp/arith.js Loader [instantiate] tmp/arith.js Analyzing ModuleRecord key('tmp/arith.js') Dependencies: 1 modules module('tmp/test001.js') Import: 0 entries Export: 1 entries [Local] export('add'), local('add') Loader [resolve] tmp/test001.js ===== tmp/bad.js import "tmp/not-found.js" = result $ Tools/Scripts/run-jsc -m tmp/bad.js Running 1 time(s): DYLD_FRAMEWORK_PATH=/Users/yusukesuzuki/development/Modules/WebKitBuild/async/Release /Users/yusukesuzuki/development/Modules/WebKitBuild/async/Release/jsc -m tmp/bad.js Loader [resolve] tmp/bad.js Loader [fetch] tmp/bad.js Loader [translate] tmp/bad.js Loader [instantiate] tmp/bad.js Analyzing ModuleRecord key('tmp/bad.js') Dependencies: 1 modules module('tmp/not-found.js') Import: 0 entries Export: 0 entries Loader [resolve] tmp/not-found.js Loader [fetch] tmp/not-found.js Could not open file: tmp/not-found.js Exception: Error: Could not open file 'tmp/not-found.js'. fetch@[native code] requestFetch@[native code] requestTranslate@[native code] requestInstantiate@[native code] requestResolveDependencies@[native code] [native code] promiseReactionJob@[native code] ===== tmp/bad2.js import "tmp/bad-syntax.js" ===== tmp/bad-syntax.js function = result $ Tools/Scripts/run-jsc -m tmp/bad2.js Running 1 time(s): DYLD_FRAMEWORK_PATH=/Users/yusukesuzuki/development/Modules/WebKitBuild/async/Release /Users/yusukesuzuki/development/Modules/WebKitBuild/async/Release/jsc -m tmp/bad2.js Loader [resolve] tmp/bad2.js Loader [fetch] tmp/bad2.js Loader [translate] tmp/bad2.js Loader [instantiate] tmp/bad2.js Analyzing ModuleRecord key('tmp/bad2.js') Dependencies: 1 modules module('tmp/bad-syntax.js') Import: 0 entries Export: 0 entries Loader [resolve] tmp/bad-syntax.js Loader [fetch] tmp/bad-syntax.js Loader [translate] tmp/bad-syntax.js Loader [instantiate] tmp/bad-syntax.js Exception: SyntaxError: Unexpected end of script parseModule@[native code] instantiation@[native code] commitInstantiated@[native code] [native code] promiseReactionJob@[native code]
Comment on attachment 259588 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259588&action=review r=me > Source/JavaScriptCore/builtins/ModuleLoaderObject.js:404 > + // Loader.resolve hook point. Maybe it would nice to also say: "resolve: moduleName => Promise(moduleKey)" This is slightly redundant with the code, but I think it makes these hook points explicitly clear.
Comment on attachment 259588 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259588&action=review Thanks! >> Source/JavaScriptCore/builtins/ModuleLoaderObject.js:404 >> + // Loader.resolve hook point. > > Maybe it would nice to also say: "resolve: moduleName => Promise(moduleKey)" > This is slightly redundant with the code, but I think it makes these hook points explicitly clear. Agreed. The name "resolve" is slightly vague. Adding these notes makes the code more readable. When completing the prototype, we have the chance to rename these APIs more descriptive ones :)
Committed r188894: <http://trac.webkit.org/changeset/188894>