diff --git a/docs/painless/painless-guide/painless-debugging.asciidoc b/docs/painless/painless-guide/painless-debugging.asciidoc index ef1045b3ac875..ce383ebf72c27 100644 --- a/docs/painless/painless-guide/painless-debugging.asciidoc +++ b/docs/painless/painless-guide/painless-debugging.asciidoc @@ -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 <> to see a list of diff --git a/docs/reference/scripting/using.asciidoc b/docs/reference/scripting/using.asciidoc index 9d056415d8f76..e9f85801dcfa0 100644 --- a/docs/reference/scripting/using.asciidoc +++ b/docs/reference/scripting/using.asciidoc @@ -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 <> 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. diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java index 6139e66160ee6..280783921d5d1 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java @@ -58,14 +58,15 @@ public interface PainlessScript { default ScriptException convertToScriptException(Throwable t, Map> extraMetadata) { // create a script stack: this is just the script portion List 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 @@ default ScriptException convertToScriptException(Throwable t, Map> entry : extraMetadata.entrySet()) { scriptException.addMetadata(entry.getKey(), entry.getValue()); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java index 8eee37e888f59..0b844ad7cf14e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java @@ -439,14 +439,15 @@ private CompilerSettings buildCompilerSettings(Map params) { private ScriptException convertToScriptException(String scriptSource, Throwable t) { // create a script stack: this is just the script portion List 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 @@ private ScriptException convertToScriptException(String scriptSource, Throwable } 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. diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/17_update_error.yml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/17_update_error.yml new file mode 100644 index 0000000000000..3d6db1b781caf --- /dev/null +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/17_update_error.yml @@ -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 } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptException.java b/server/src/main/java/org/elasticsearch/script/ScriptException.java index 2d5d6841618e5..d538ef3554cdf 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptException.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptException.java @@ -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 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 scriptStack, String script, String lang) { + public ScriptException(String message, Throwable cause, List 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 scriptStack, String script, String lang) { + this(message, cause, scriptStack, script, lang, null); } /** @@ -78,6 +89,11 @@ public ScriptException(StreamInput in) throws IOException { 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 void writeTo(StreamOutput out) throws IOException { 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 @@ protected void metadataToXContent(XContentBuilder builder, Params params) throws 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 String getLang() { 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 String toJsonString() { 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); + } + } } diff --git a/server/src/test/java/org/elasticsearch/script/ScriptExceptionTests.java b/server/src/test/java/org/elasticsearch/script/ScriptExceptionTests.java index 929c14e45e2de..6d5eac06f95c8 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptExceptionTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptExceptionTests.java @@ -35,31 +35,45 @@ /** 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 void testJsonOutput() { 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 void testImmutableStack() { }); } - /** 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");