honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-24 17:09:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Scripting: Add char position of script errors (#51069) (#51266)

Browse Source 
Add the character position of a scripting error to error responses.

The contents of the `position` field are experimental and subject to
change.  Currently, `offset` refers to the character location where the
error was encountered, `start` and `end` define a range of characters
that contain the error.

eg.
```
{
  "error": {
    "root_cause": [
      {
        "type": "script_exception",
        "reason": "runtime error",
        "script_stack": [
          "y = x;",
          "     ^---- HERE"
        ],
        "script": "def x = new ArrayList(); Map y = x;",
        "lang": "painless",
        "position": {
          "offset": 33,
          "start": 29,
          "end": 35
        }
      }
```

Refs: #50993
This commit is contained in:
Stuart Tettemer 2020-01-21 13:45:59 -07:00 committed by GitHub
parent 5d4bbdcc50
commit 41c15b438d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 149 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
docs/painless/painless-guide/painless-debugging.asciidoc
View File

@ -50,7 +50,7 @@ Which shows that the class of `doc.first` is
"status": 400
}
---------------------------------------------------------
// TESTRESPONSE[s/\.\.\./"script_stack": $body.error.script_stack, "script": $body.error.script, "lang": $body.error.lang, "caused_by": $body.error.caused_by, "root_cause": $body.error.root_cause, "reason": $body.error.reason/]
// TESTRESPONSE[s/\.\.\./"script_stack": $body.error.script_stack, "script": $body.error.script, "lang": $body.error.lang, "position": $body.error.position, "caused_by": $body.error.caused_by, "root_cause": $body.error.root_cause, "reason": $body.error.reason/]
You can use the same trick to see that `_source` is a `LinkedHashMap`
in the `_update` API:
@ -85,7 +85,7 @@ The response looks like:
}
---------------------------------------------------------
// TESTRESPONSE[s/"root_cause": \.\.\./"root_cause": $body.error.root_cause/]
// TESTRESPONSE[s/\.\.\./"script_stack": $body.error.caused_by.script_stack, "script": $body.error.caused_by.script, "lang": $body.error.caused_by.lang, "caused_by": $body.error.caused_by.caused_by, "reason": $body.error.caused_by.reason/]
// TESTRESPONSE[s/\.\.\./"script_stack": $body.error.caused_by.script_stack, "script": $body.error.caused_by.script, "lang": $body.error.caused_by.lang, "position": $body.error.caused_by.position, "caused_by": $body.error.caused_by.caused_by, "reason": $body.error.caused_by.reason/]
// TESTRESPONSE[s/"to_string": ".+"/"to_string": $body.error.caused_by.to_string/]
Once you have a class you can go to <<painless-api-reference>> to see a list of

11
docs/reference/scripting/using.asciidoc
View File

@ -241,3 +241,14 @@ NOTE: The size of scripts is limited to 65,535 bytes. This can be
changed by setting `script.max_size_in_bytes` setting to increase that soft
limit, but if scripts are really large then a
<<modules-scripting-engine,native script engine>> should be considered.
[float]
[[modules-scripting-errors]]
=== Script errors
Elasticsearch returns error details when there is a compliation or runtime
exception. The contents of this response are useful for tracking down the
problem.
experimental[]
The contents of `position` are experimental and subject to change.

10
modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java
View File

@ -58,14 +58,15 @@ public interface PainlessScript {
default ScriptException convertToScriptException(Throwable t, Map<String, List<String>> extraMetadata) {
// create a script stack: this is just the script portion
List<String> scriptStack = new ArrayList<>();
ScriptException.Position pos = null;
for (StackTraceElement element : t.getStackTrace()) {
if (WriterConstants.CLASS_NAME.equals(element.getClassName())) {
// found the script portion
int offset = element.getLineNumber();
if (offset == -1) {
int originalOffset = element.getLineNumber();
if (originalOffset == -1) {
scriptStack.add("<<< unknown portion of script >>>");
} else {
offset--; // offset is 1 based, line numbers must be!
int offset = --originalOffset; // offset is 1 based, line numbers must be!
int startOffset = getPreviousStatement(offset);
if (startOffset == -1) {
assert false; // should never happen unless we hit exc in ctor prologue...
@ -84,6 +85,7 @@ public interface PainlessScript {
}
pointer.append("^---- HERE");
scriptStack.add(pointer.toString());
pos = new ScriptException.Position(originalOffset, startOffset, endOffset);
}
break;
// but filter our own internal stacks (e.g. indy bootstrap)
@ -91,7 +93,7 @@ public interface PainlessScript {
scriptStack.add(element.toString());
}
}
ScriptException scriptException = new ScriptException("runtime error", t, scriptStack, getName(), PainlessScriptEngine.NAME);
ScriptException scriptException = new ScriptException("runtime error", t, scriptStack, getName(), PainlessScriptEngine.NAME, pos);
for (Map.Entry<String, List<String>> entry : extraMetadata.entrySet()) {
scriptException.addMetadata(entry.getKey(), entry.getValue());
}

10
modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java
View File

@ -439,14 +439,15 @@ public final class PainlessScriptEngine implements ScriptEngine {
private ScriptException convertToScriptException(String scriptSource, Throwable t) {
// create a script stack: this is just the script portion
List<String> scriptStack = new ArrayList<>();
ScriptException.Position pos = null;
for (StackTraceElement element : t.getStackTrace()) {
if (WriterConstants.CLASS_NAME.equals(element.getClassName())) {
// found the script portion
int offset = element.getLineNumber();
if (offset == -1) {
int originalOffset = element.getLineNumber();
if (originalOffset == -1) {
scriptStack.add("<<< unknown portion of script >>>");
} else {
offset--; // offset is 1 based, line numbers must be!
int offset = --originalOffset; // offset is 1 based, line numbers must be!
int startOffset = getPreviousStatement(offset);
int endOffset = getNextStatement(scriptSource, offset);
StringBuilder snippet = new StringBuilder();
@ -467,11 +468,12 @@ public final class PainlessScriptEngine implements ScriptEngine {
}
pointer.append("^---- HERE");
scriptStack.add(pointer.toString());
pos = new ScriptException.Position(originalOffset, startOffset, endOffset);
}
break;
}
}
throw new ScriptException("compile error", t, scriptStack, scriptSource, PainlessScriptEngine.NAME);
throw new ScriptException("compile error", t, scriptStack, scriptSource, PainlessScriptEngine.NAME, pos);
}
// very simple heuristic: +/- 25 chars. can be improved later.

15
modules/lang-painless/src/test/resources/rest-api-spec/test/painless/17_update_error.yml Normal file
View File

@ -0,0 +1,15 @@
---
"Script errors contain position":
- skip:
version: " - 7.7.0"
reason: "position introduced in 7.7"
- do:
catch: /compile error/
put_script:
id: "1"
context: "score"
body: { "script": {"lang": "painless", "source": "_score * foo bar + doc['myParent.weight'].value"} }
- match: { error.root_cause.0.position.offset: 13 }
- match: { error.root_cause.0.position.start: 0 }
- match: { error.root_cause.0.position.end: 38 }

85
server/src/main/java/org/elasticsearch/script/ScriptException.java
View File

@ -1,6 +1,7 @@
package org.elasticsearch.script;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
@ -51,6 +52,7 @@ public class ScriptException extends ElasticsearchException {
private final List<String> scriptStack;
private final String script;
private final String lang;
private final Position pos;
/**
* Create a new ScriptException.
@ -61,13 +63,22 @@ public class ScriptException extends ElasticsearchException {
* Must not be {@code null}, but can be empty (though this should be avoided if possible).
* @param script Identifier for which script failed. Must not be {@code null}.
* @param lang Scripting engine language, such as "painless". Must not be {@code null}.
* @throws NullPointerException if any parameters are {@code null}.
* @param pos Position of error within script, may be {@code null}.
* @throws NullPointerException if any parameters are {@code null} except pos.
*/
public ScriptException(String message, Throwable cause, List<String> scriptStack, String script, String lang) {
public ScriptException(String message, Throwable cause, List<String> scriptStack, String script, String lang, Position pos) {
super(Objects.requireNonNull(message), Objects.requireNonNull(cause));
this.scriptStack = Collections.unmodifiableList(Objects.requireNonNull(scriptStack));
this.script = Objects.requireNonNull(script);
this.lang = Objects.requireNonNull(lang);
this.pos = pos;
}
/**
* Create a new ScriptException with null Position.
*/
public ScriptException(String message, Throwable cause, List<String> scriptStack, String script, String lang) {
this(message, cause, scriptStack, script, lang, null);
}
/**
@ -78,6 +89,11 @@ public class ScriptException extends ElasticsearchException {
scriptStack = Arrays.asList(in.readStringArray());
script = in.readString();
lang = in.readString();
if (in.getVersion().onOrAfter(Version.V_7_7_0) && in.readBoolean()) {
pos = new Position(in);
} else {
pos = null;
}
}
@Override
@ -86,6 +102,14 @@ public class ScriptException extends ElasticsearchException {
out.writeStringArray(scriptStack.toArray(new String[0]));
out.writeString(script);
out.writeString(lang);
if (out.getVersion().onOrAfter(Version.V_7_7_0)) {
if (pos == null) {
out.writeBoolean(false);
} else {
out.writeBoolean(true);
pos.writeTo(out);
}
}
}
@Override
@ -93,6 +117,9 @@ public class ScriptException extends ElasticsearchException {
builder.field("script_stack", scriptStack);
builder.field("script", script);
builder.field("lang", lang);
if (pos != null) {
pos.toXContent(builder, params);
}
}
/**
@ -119,6 +146,13 @@ public class ScriptException extends ElasticsearchException {
return lang;
}
/**
* Returns the position of the error.
*/
public Position getPos() {
return pos;
}
/**
* Returns a JSON version of this exception for debugging.
*/
@ -138,4 +172,51 @@ public class ScriptException extends ElasticsearchException {
public RestStatus status() {
return RestStatus.BAD_REQUEST;
}
public static class Position {
public final int offset;
public final int start;
public final int end;
public Position(int offset, int start, int end) {
this.offset = offset;
this.start = start;
this.end = end;
}
Position(StreamInput in) throws IOException {
offset = in.readInt();
start = in.readInt();
end = in.readInt();
}
void writeTo(StreamOutput out) throws IOException {
out.writeInt(offset);
out.writeInt(start);
out.writeInt(end);
}
void toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject("position");
builder.field("offset", offset);
builder.field("start", start);
builder.field("end", end);
builder.endObject();
}
@Override
public boolean equals(Object o) {
if (this == o)
return true;
if (o == null || getClass() != o.getClass())
return false;
Position position = (Position) o;
return offset == position.offset && start == position.start && end == position.end;
}
@Override
public int hashCode() {
return Objects.hash(offset, start, end);
}
}
}

35
server/src/test/java/org/elasticsearch/script/ScriptExceptionTests.java
View File

@ -35,31 +35,45 @@ import java.util.List;
/** Simple tests for {@link ScriptException} */
public class ScriptExceptionTests extends ESTestCase {
/** ensure we can round trip in serialization */
public void testRoundTrip() throws IOException {
ScriptException e = new ScriptException("messageData", new Exception("causeData"), Arrays.asList("stack1", "stack2"),
ScriptException e = new ScriptException("messageData", new Exception("causeData"), Arrays.asList("stack1", "stack2"),
"sourceData", "langData");
ByteArrayOutputStream bytes = new ByteArrayOutputStream();
StreamOutput output = new DataOutputStreamOutput(new DataOutputStream(bytes));
e.writeTo(output);
output.close();
StreamInput input = new InputStreamStreamInput(new ByteArrayInputStream(bytes.toByteArray()));
ScriptException e2 = new ScriptException(input);
input.close();
assertEquals(e.getMessage(), e2.getMessage());
assertEquals(e.getScriptStack(), e2.getScriptStack());
assertEquals(e.getScript(), e2.getScript());
assertEquals(e.getLang(), e2.getLang());
assertNull(e.getPos());
// Ensure non-null position also works
e = new ScriptException(e.getMessage(), e.getCause(), e.getScriptStack(), e.getScript(), e.getLang(),
new ScriptException.Position(1, 0, 2));
bytes = new ByteArrayOutputStream();
output = new DataOutputStreamOutput(new DataOutputStream(bytes));
e.writeTo(output);
output.close();
input = new InputStreamStreamInput(new ByteArrayInputStream(bytes.toByteArray()));
e2 = new ScriptException(input);
input.close();
assertEquals(e.getPos(), e2.getPos());
}
/** Test that our elements are present in the json output */
public void testJsonOutput() {
ScriptException e = new ScriptException("messageData", new Exception("causeData"), Arrays.asList("stack1", "stack2"),
"sourceData", "langData");
ScriptException e = new ScriptException("messageData", new Exception("causeData"), Arrays.asList("stack1", "stack2"),
"sourceData", "langData", new ScriptException.Position(2, 1, 3));
String json = e.toJsonString();
assertTrue(json.contains(e.getMessage()));
assertTrue(json.contains(e.getCause().getMessage()));
@ -67,6 +81,9 @@ public class ScriptExceptionTests extends ESTestCase {
assertTrue(json.contains("stack2"));
assertTrue(json.contains(e.getScript()));
assertTrue(json.contains(e.getLang()));
assertTrue(json.contains("1"));
assertTrue(json.contains("2"));
assertTrue(json.contains("3"));
}
/** ensure the script stack is immutable */
@ -78,7 +95,7 @@ public class ScriptExceptionTests extends ESTestCase {
});
}
/** ensure no parameters can be null */
/** ensure no parameters can be null except pos*/
public void testNoLeniency() {
expectThrows(NullPointerException.class, () -> {
new ScriptException(null, new Exception(), Collections.emptyList(), "a", "b");