Merge pull request #78 from Distributive-Network/wes/better-stacks

Improve error stacks

Author: zollqir

python/pythonmonkey/cli/pmjs.py

@@ -6,7 +6,7 @@
 import sys, os, readline, signal, getopt
 import pythonmonkey as pm
 globalThis = pm.eval("globalThis")
-evalOptions = { 'strict': False }
+evalOpts = { 'filename': __file__, 'fromPythonFrame': True, 'strict': False }
 
 if (os.getenv('PMJS_PATH')):
     requirePath = list(map(os.path.abspath, os.getenv('PMJS_PATH').split(':')))
@@ -40,11 +40,17 @@
     pythonCmd.serial = 0;
     return;
   }
-
-  if (arguments[0] === 'from' || arguments[0] === 'import')
+  try {
+    if (arguments[0] === 'from' || arguments[0] === 'import')
     return python.exec(cmd);
 
-  const retval = python.eval(cmd);
+    const retval = python.eval(cmd);
+  }
+  catch(error) {
+    globalThis._error = error;
+    return util.inspect(error);
+  }
+  
   pythonCmd.serial = (pythonCmd.serial || 0) + 1;
   globalThis['$' + pythonCmd.serial] = retval;
   python.stdout.write('$' + pythonCmd.serial + ' = ');
@@ -127,7 +133,7 @@
     return util.inspect(error);
   }
 }
-""");
+""", evalOpts);
 
 def repl():
     """
@@ -319,21 +325,20 @@ def main():
             print(pm.__version__)
             sys.exit()
         elif o in ("--use-strict"):
-            evalOptions['strict'] = True
+            evalOpts['strict'] = True
         elif o in ("-h", "--help"):
             usage()
             sys.exit()
         elif o in ("-i", "--interactive"):
             forceRepl = True
         elif o in ("-e", "--eval"):
-            pm.eval(a, evalOptions)
+            pm.eval(a, evalOpts)
             enterRepl = False
         elif o in ("-p", "--print"):
-            print(pm.eval(a, evalOptions))
+            print(pm.eval(a, evalOpts))
             enterRepl = False
         elif o in ("-r", "--require"):
             globalThis.require(a)
-            #            pm.eval('require')(a)
         else:
             assert False, "unhandled option"
 

python/pythonmonkey/pythonmonkey.pyi

@@ -5,8 +5,19 @@ stub file for type hints & documentations for the native module
 
 import typing as _typing
 
+class EvalOptions(_typing.TypedDict, total=False):
+    filename: str
+    lineno: int
+    column: int
+    mutedErrors: bool
+    noScriptRval: bool
+    selfHosting: bool
+    strict: bool
+    module: bool
+    fromPythonFrame: bool
+
 # pylint: disable=redefined-builtin
-def eval(code: str, /) -> _typing.Any:
+def eval(code: str, evalOpts: EvalOptions = {}, /) -> _typing.Any:
     """
     JavaScript evaluator in Python
     """

python/pythonmonkey/require.py

@@ -40,10 +40,11 @@
     "node_modules"
   )
 )
+evalOpts = { 'filename': __file__, 'fromPythonFrame': True }
 
 # Add some python functions to the global python object for code in this file to use.
-globalThis = pm.eval("globalThis;")
-pm.eval("globalThis.python = { pythonMonkey: {}, stdout: {}, stderr: {} }")
+globalThis = pm.eval("globalThis;", evalOpts)
+pm.eval("globalThis.python = { pythonMonkey: {}, stdout: {}, stderr: {} }", evalOpts);
 globalThis.pmEval = pm.eval
 globalThis.python.pythonMonkey.dir = os.path.dirname(__file__)
 #globalThis.python.pythonMonkey.version = pm.__version__
@@ -59,15 +60,15 @@
 globalThis.python.exec = exec
 globalThis.python.getenv = os.getenv
 globalThis.python.paths  = ':'.join(sys.path)
-pm.eval("python.paths = python.paths.split(':');"); # fix when pm supports arrays
+pm.eval("python.paths = python.paths.split(':');", evalOpts); # fix when pm supports arrays
 
 globalThis.python.exit = pm.eval("""'use strict';
 (exit) => function pythonExitWrapper(exitCode) {
   if (typeof exitCode === 'number')
     exitCode = BigInt(Math.floor(exitCode));
   exit(exitCode);
 }
-""")(sys.exit);
+""", evalOpts)(sys.exit);
 
 # bootstrap is effectively a scoping object which keeps us from polluting the global JS scope.
 # The idea is that we hold a reference to the bootstrap object in Python-load, for use by the
@@ -78,7 +79,7 @@
 
 const bootstrap = {
   modules: {
-    vm: { runInContext: eval },
+    vm: {},
     'ctx-module': {},
   },
 }
@@ -92,7 +93,7 @@
   throw new Error('module not found: ' + mid);
 }
 
-bootstrap.modules.vm.runInContext_broken = function runInContext(code, _unused_contextifiedObject, options)
+bootstrap.modules.vm.runInContext = function runInContext(code, _unused_contextifiedObject, options)
 {
   var evalOptions = {};
 
@@ -169,7 +170,7 @@
 globalThis.bootstrap = bootstrap;
 
 return bootstrap;
-})(globalThis.python)""")
+})(globalThis.python)""", evalOpts)
 
 def statSync_inner(filename: str) -> Union[Dict[str, int], bool]:
     """
@@ -208,15 +209,18 @@ def existsSync(filename: str) -> bool:
 # require and exports symbols injected from the bootstrap object above. Current PythonMonkey bugs
 # prevent us from injecting names properly so they are stolen from trail left behind in the global
 # scope until that can be fixed.
+#
+# lineno should be -5 but jsapi 102 uses unsigned line numbers, so we take the newlines out of the
+# wrapper prologue to make stack traces line up.
 with open(node_modules + "/ctx-module/ctx-module.js", "r") as ctxModuleSource:
     initCtxModule = pm.eval("""'use strict';
 (function moduleWrapper_forCtxModule(broken_require, broken_exports)
 {
   const require = bootstrap.require;
   const exports = bootstrap.modules['ctx-module'];
-""" + ctxModuleSource.read() + """
+""".replace("\n", " ") + "\n" + ctxModuleSource.read() + """
 })
-""");
+""", { 'filename': node_modules + "/ctx-module/ctx-module.js", 'lineno': 0 });
 #broken initCtxModule(bootstrap.require, bootstrap.modules['ctx-module'].exports)
 initCtxModule();
 
@@ -298,7 +302,7 @@ def _createRequireInner(*args):
     module.require.path.splice(module.require.path.length, 0, ...(extraPaths.split(':')));
 
   return module.require;
-})""")(*args)
+})""", evalOpts)(*args)
 
 def createRequire(filename, extraPaths: Union[List[str], Literal[False]] = False, isMain = False):
     """
@@ -331,7 +335,7 @@ def runProgramModule(filename, argv, extraPaths=[]):
     globalThis.__filename = fullFilename;
     globalThis.__dirname = os.path.dirname(fullFilename);
     with open(fullFilename, encoding="utf-8", mode="r") as mainModuleSource:
-        pm.eval(mainModuleSource.read())
+        pm.eval(mainModuleSource.read(), {'filename': fullFilename})
 
 def require(moduleIdentifier: str):
     # Retrieve the caller’s filename from the call stack

src/modules/pythonmonkey/pythonmonkey.cc

@@ -5,7 +5,7 @@
  * @version 0.1
  * @date 2023-03-29
  *
- * @copyright Copyright (c) 2023
+ * @copyright Copyright (c) 2023 Distributive Corp.
  *
  */
 
@@ -188,7 +188,7 @@ static PyObject *eval(PyObject *self, PyObject *args) {
 
   JSAutoRealm ar(GLOBAL_CX, *global);
   JS::CompileOptions options (GLOBAL_CX);
-  options.setFileAndLine("@evaluate", 1)
+  options.setFileAndLine("evaluate", 1)
   .setIsRunOnce(true)
   .setNoScriptRval(false)
   .setIntroductionType("pythonmonkey eval");
@@ -206,9 +206,29 @@ static PyObject *eval(PyObject *self, PyObject *args) {
     if (getEvalOption(evalOptions, "selfHosting", &b)) options.setSelfHostingMode(b);
     if (getEvalOption(evalOptions, "strict", &b)) if (b) options.setForceStrictMode();
     if (getEvalOption(evalOptions, "module", &b)) if (b) options.setModule();
-  }
 
-  // initialize JS context
+    if (getEvalOption(evalOptions, "fromPythonFrame", &b) && b) {
+#if PY_VERSION_HEX >= 0x03090000
+      PyFrameObject *frame = PyEval_GetFrame();
+      if (frame && !getEvalOption(evalOptions, "lineno", &l)) {
+        options.setLine(PyFrame_GetLineNumber(frame));
+      } /* lineno */
+#endif
+#if 0 && (PY_VERSION_HEX >= 0x030a0000) && (PY_VERSION_HEX < 0x030c0000)
+      PyObject *filename = PyDict_GetItemString(frame->f_builtins, "__file__");
+#elif (PY_VERSION_HEX >= 0x030c0000)
+      PyObject *filename = PyDict_GetItemString(PyFrame_GetGlobals(frame), "__file__");
+#else
+      PyObject *filename = NULL;
+#endif
+      if (!getEvalOption(evalOptions, "filename", &s)) {
+        if (filename && PyUnicode_Check(filename)) {
+          options.setFile(PyUnicode_AsUTF8(filename));
+        }
+      } /* filename */
+    } /* fromPythonFrame */
+  } /* eval options */
+    // initialize JS context
   JS::SourceText<mozilla::Utf8Unit> source;
   if (!source.init(GLOBAL_CX, code->getValue(), strlen(code->getValue()), JS::SourceOwnership::Borrowed)) {
     setSpiderMonkeyException(GLOBAL_CX);
@@ -230,13 +250,13 @@ static PyObject *eval(PyObject *self, PyObject *args) {
   }
 
   // TODO: Find a better way to destroy the root when necessary (when the returned Python object is GCed).
-  js::ESClass cls = js::ESClass::Other; // placeholder if `rval` is not a JSObject
+  js::ESClass cls = js::ESClass::Other;   // placeholder if `rval` is not a JSObject
   if (rval->isObject()) {
     JS::GetBuiltinClass(GLOBAL_CX, JS::RootedObject(GLOBAL_CX, &rval->toObject()), &cls);
   }
-  bool rvalIsFunction = cls == js::ESClass::Function; // function object
-  bool rvalIsString = rval->isString() || cls == js::ESClass::String; // string primitive or boxed String object
-  if (!(rvalIsFunction || rvalIsString)) {  // rval may be a JS function or string which must be kept alive.
+  bool rvalIsFunction = cls == js::ESClass::Function;   // function object
+  bool rvalIsString = rval->isString() || cls == js::ESClass::String;   // string primitive or boxed String object
+  if (!(rvalIsFunction || rvalIsString)) {   // rval may be a JS function or string which must be kept alive.
     delete rval;
   }
 
@@ -278,7 +298,7 @@ struct PyModuleDef pythonmonkey =
 {
   PyModuleDef_HEAD_INIT,
   "pythonmonkey",                                   /* name of module */
-  "A module for python to JS interoperability", /* module documentation, may be NULL */
+  "A module for python to JS interoperability",   /* module documentation, may be NULL */
   -1,                                           /* size of per-interpreter state of the module, or -1 if the module keeps state in global variables. */
   PythonMonkeyMethods
 };
@@ -302,29 +322,29 @@ static bool setTimeout(JSContext *cx, unsigned argc, JS::Value *vp) {
 
   // Get the function to be executed
   // FIXME (Tom Tang): memory leak, not free-ed
-  JS::RootedObject *thisv = new JS::RootedObject(cx, JS::GetNonCCWObjectGlobal(&args.callee())); // HTML spec requires `thisArg` to be the global object
+  JS::RootedObject *thisv = new JS::RootedObject(cx, JS::GetNonCCWObjectGlobal(&args.callee()));   // HTML spec requires `thisArg` to be the global object
   JS::RootedValue *jobArg = new JS::RootedValue(cx, jobArgVal);
   // `setTimeout` allows passing additional arguments to the callback, as spec-ed
-  if (args.length() > 2) { // having additional arguments
+  if (args.length() > 2) {   // having additional arguments
     // Wrap the job function into a bound function with the given additional arguments
     //    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind
     JS::RootedVector<JS::Value> bindArgs(cx);
-    (void)bindArgs.append(JS::ObjectValue(**thisv)); /** @todo XXXwg handle return value */
+    (void)bindArgs.append(JS::ObjectValue(**thisv));   /** @todo XXXwg handle return value */
     for (size_t j = 2; j < args.length(); j++) {
-      (void)bindArgs.append(args[j]); /** @todo XXXwg handle return value */
+      (void)bindArgs.append(args[j]);   /** @todo XXXwg handle return value */
     }
     JS::RootedObject jobArgObj = JS::RootedObject(cx, &jobArgVal.toObject());
-    JS_CallFunctionName(cx, jobArgObj, "bind", JS::HandleValueArray(bindArgs), jobArg); // jobArg = jobArg.bind(thisv, ...bindArgs)
+    JS_CallFunctionName(cx, jobArgObj, "bind", JS::HandleValueArray(bindArgs), jobArg);   // jobArg = jobArg.bind(thisv, ...bindArgs)
   }
   // Convert to a Python function
   PyObject *job = pyTypeFactory(cx, thisv, jobArg)->getPyObject();
 
   // Get the delay time
   //  JS `setTimeout` takes milliseconds, but Python takes seconds
-  double delayMs = 0; // use value of 0 if the delay parameter is omitted
-  if (args.hasDefined(1)) { JS::ToNumber(cx, args[1], &delayMs); } // implicitly do type coercion to a `number`
-  if (delayMs < 0) { delayMs = 0; } // as spec-ed
-  double delaySeconds = delayMs / 1000; // convert ms to s
+  double delayMs = 0;   // use value of 0 if the delay parameter is omitted
+  if (args.hasDefined(1)) { JS::ToNumber(cx, args[1], &delayMs); }   // implicitly do type coercion to a `number`
+  if (delayMs < 0) { delayMs = 0; }   // as spec-ed
+  double delaySeconds = delayMs / 1000;   // convert ms to s
 
   // Schedule job to the running Python event-loop
   PyEventLoop loop = PyEventLoop::getRunningLoop();
@@ -355,7 +375,7 @@ static bool clearTimeout(JSContext *cx, unsigned argc, JS::Value *vp) {
   // Retrieve the AsyncHandle by `timeoutID`
   int32_t timeoutID = timeoutIdArg.toInt32();
   AsyncHandle *handle = AsyncHandle::fromId((uint32_t)timeoutID);
-  if (!handle) return true; // does nothing on invalid timeoutID
+  if (!handle) return true;   // does nothing on invalid timeoutID
 
   // Cancel this job on Python event-loop
   handle->cancel();
@@ -424,7 +444,7 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void)
   // In https://hg.mozilla.org/releases/mozilla-esr102/file/3b574e1/js/src/jit/CacheIR.cpp#l317, trying to use the callback returned by `js::GetDOMProxyShadowsCheck()` even it's unset (nullptr)
   // Temporarily solved by explicitly setting the `domProxyShadowsCheck` callback here
   JS::SetDOMProxyInformation(nullptr,
-    [](JSContext *, JS::HandleObject, JS::HandleId) { // domProxyShadowsCheck
+    [](JSContext *, JS::HandleObject, JS::HandleId) {   // domProxyShadowsCheck
       return JS::DOMProxyShadowsResult::ShadowCheckFailed;
     }, nullptr);
 

tests/js/require-module-stack.bash.failing

@@ -0,0 +1,29 @@
+#! /bin/bash
+#
+# @file         require-module-stack.bash
+#               A peter-jr test which tests that requiring a module yields JS stacks with the correct 
+#               filename and line number information.
+#
+#               NOTE: This test currently fails because the stack coming back through python has
+#                     underscores instead of slashes in the filename
+#
+# @author       Wes Garland, [email protected]
+# @date         July 2023
+
+set -u
+set -o pipefail
+
+panic()
+{
+  echo "FAIL: $*" >&2
+  exit 2
+}
+
+cd `dirname "$0"` || panic "could not change to test directory"
+
+if "${PMJS:-../../pmjs}" -r ./throw-filename ./program.js \
+   | grep '^@/home/wes/git/pythonmonkey2/tests/js/throw-filename.js:9:9$'; then
+  echo 'pass'
+fi
+
+panic "did not find correct stack info"

tests/js/throw-filename.js

@@ -0,0 +1,14 @@
+/**
+ * @file        throw-filename.js
+ *              A helper that throws when loaded, so that we can test that loading things throws with the right filename.
+ * @author      Wes Garland, [email protected]
+ * @date        July 2023
+ */
+try
+{
+  throw new Error("lp0 on fire");
+}
+catch(error)
+{
+  console.log(error.stack)
+}