Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Script engines should be aware of the script name #6653

Closed
dakrone opened this issue Jun 30, 2014 · 3 comments
Closed

Script engines should be aware of the script name #6653

dakrone opened this issue Jun 30, 2014 · 3 comments
Assignees
@jdconrad
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement

Comments

@dakrone
Copy link
Member

dakrone commented Jun 30, 2014

It would be nice if we could access the name of the script from the script engine framework so it could be logged when errors occur. This would be especially helpful for additional logging for users with dynamic scripting disabled.
@dakrone dakrone mentioned this issue Jun 30, 2014
Merged
@javanna
Copy link
Member

javanna commented Mar 19, 2015

Heya @dakrone are you working on this? If not can you elaborate on what you mean? Where should the script name be available but it isn't?

@javanna javanna added the :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache label Mar 19, 2015
@dakrone
Copy link
Member Author

dakrone commented Mar 20, 2015

@javanna when exceptions are thrown in a script, it's nice to know in the logs which script (if it came from on disk) the error came from, I believe right now we use:

private String generateScriptName() {
  return "Script" + counter.incrementAndGet() + ".groovy";
}

Which works for dynamic scripts (maybe we want to rename it "dynamic-script1.groovy" or something to be extra clear. The on-disk ones should have their name clearly in the exception/logging.

@dakrone dakrone assigned jdconrad and unassigned dakrone Apr 13, 2015
@jdconrad jdconrad mentioned this issue Apr 21, 2015
Merged
jdconrad added a commit to jdconrad/elasticsearch that referenced this issue Jun 1, 2015
@jdconrad
Scripting: Add script name to error messages
4570fc5 
Modified ScriptEngineService to pass in a CompiledScript object
to several methods with newly added name and type member variables.
This can in turn be used to give better scripting error messages
with the type of script used and the name of the script.

Required slight modifications to the caching mechanism.

closes elastic#6653
@jdconrad jdconrad mentioned this issue Jun 1, 2015
Closed
jdconrad added a commit to jdconrad/elasticsearch that referenced this issue Jun 3, 2015
@jdconrad
Scripting: Add script name to error messages
f022622 
Modified ScriptEngineService to pass in a CompiledScript object
to several methods with newly added name and type member variables.
This can in turn be used to give better scripting error messages
with the type of script used and the name of the script.

Required slight modifications to the caching mechanism.

closes elastic#6653
closes elastic#11449
@jdconrad
Copy link
Contributor

@rjernst Would you please take a look when you get a chance? Thanks!

jdconrad added a commit to jdconrad/elasticsearch that referenced this issue Jul 11, 2015
@jdconrad
Add script type and script name to error messages
f46ba22 
Modified ScriptEngineService to pass in a CompiledScript object
with newly added name and type member variables.
This can in turn be used to give better scripting error messages
with the type of script used and the name of the script.

Required slight modifications to the caching mechanism.

Note that this does not enforce good behavior in that plugins will
have to write exceptions that also output the name of the script
in order to be effective. There was no way to wrap the script
methods in a try/catch block properly further up the chain because
many have script-like objects passed back that can be run at a
later time.

closes elastic#6653
closes elastic#11449
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jdconrad jdconrad

Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add script type and script name to error messages jdconrad/elasticsearch
3 participants
@dakrone @javanna @jdconrad