Skip to content

Commit

Permalink
Fix: add file name in stack traces for compiled classes
Browse files Browse the repository at this point in the history
In interpreter mode, we always put the file name, but in compiled mode
we do it only in case of `compilerEnv.isGenerateDebugInfo()`. I think
that the interpreter's behavior is much more useful, so I changed the
compiler to always include the file name.

For an example of the difference, create two files:

<b>`one.js`</b>

```js
const two = require('./two');
two();
```

and <b>`two.js`</b>:

```js
module.exports = function() {
	throw new Error('hey')
}
```

Then launch the shell passing `-version 200 -opt -1 -require one.js` to
get the following, useful stack trace:

```
js: "file:/Users/andrea.bergia/src/rhino/two.js", line 2: exception from uncaught JavaScript throw: Error: hey
	at file:/Users/andrea.bergia/src/rhino/two.js:2
	at file:/Users/andrea.bergia/src/rhino/one.js:2
```

But with `-opt 0` we get:

```
js: "file:/Users/andrea.bergia/src/rhino/two.js", line 2: exception from uncaught JavaScript throw: Error: hey
	at (unknown):2 (anonymous)
	at (unknown):2
```

which isn't as useful.
  • Loading branch information
andreabergia authored and gbrail committed Dec 30, 2024
1 parent d243106 commit f87e7f3
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,7 @@ private byte[] generateCode(String rawSource) {
boolean hasFunctions = (scriptOrFnNodes.length > 1 || !hasScript);
boolean isStrictMode = scriptOrFnNodes[0].isInStrictMode();

String sourceFile = null;
if (compilerEnv.isGenerateDebugInfo()) {
sourceFile = scriptOrFnNodes[0].getSourceName();
}

String sourceFile = scriptOrFnNodes[0].getSourceName();
ClassFileWriter cfw = new ClassFileWriter(mainClassName, SUPER_CLASS_NAME, sourceFile);
cfw.addField(ID_FIELD_NAME, "I", ACC_PRIVATE);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.mozilla.javascript.tests.commonjs.module;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.io.InputStreamReader;
Expand All @@ -10,11 +12,14 @@
import java.util.Collections;
import org.junit.Test;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.RhinoException;
import org.mozilla.javascript.ScriptStackElement;
import org.mozilla.javascript.Scriptable;
import org.mozilla.javascript.ScriptableObject;
import org.mozilla.javascript.commonjs.module.Require;
import org.mozilla.javascript.commonjs.module.provider.StrongCachingModuleScriptProvider;
import org.mozilla.javascript.commonjs.module.provider.UrlModuleSourceProvider;
import org.mozilla.javascript.tests.Utils;

/**
* @author Attila Szegedi
Expand Down Expand Up @@ -137,6 +142,32 @@ public void setMainForAlreadyLoadedModule() throws Exception {
}
}

@Test
public void stackTracesAlwaysHaveFileName() {
Utils.runWithAllModes(
cx -> {
cx.setGeneratingDebug(false);
final Scriptable scope = cx.initStandardObjects();
try {
final Require require = getSandboxedRequire(cx, scope, false);
require.install(scope);
RhinoException rhinoException =
assertThrows(
RhinoException.class,
() -> require.requireMain(cx, "throw-one"));

ScriptStackElement[] stack = rhinoException.getScriptStack();
assertEquals(2, stack.length);

assertTrue(stack[0].fileName.contains("throw-two.js"));
assertTrue(stack[1].fileName.contains("throw-one.js"));
} catch (Exception e) {
throw new RuntimeException(e);
}
return null;
});
}

private Reader getReader(String name) {
return new InputStreamReader(getClass().getResourceAsStream(name));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const two = require('throw-two');
two();
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = function() {
throw new Error('hey')
}

0 comments on commit f87e7f3

Please sign in to comment.