#7245: better recovery from script engine failures

[sdedic]

Improves error handling in script engine initialization. In #7245 the graaljs script engine fails to load, but aborts the entire template operation (creation of a file using freemaker).

This PR does not fix the issue with graaljs, but improves handling of errors if a script engine fails to load or initialize. Now the error is just logged and does not abort ScriptManager init.

[matthiasblaesing]

Thank you - indeed a more focused fix, bringing a partitial solution with minimal/no negative side effects. At this point however I think it would be helpful to also have logging as in Scripting.java. Adding the catch clause for ServiceConfigurationError fixes the problem reported in #7245 (not being able to generate a HTML/JS application), but there is also no info shown, as the exception is swallowed. This also does not need an additional guard as the code that could cause the exceptions is entered at most once (disable is used as guard).

[sdedic]

OK to more logging here. I do not quite understand the part with disable: as far as I read the code, factories() can be called as part of Scripting.newBuilder().build(), each call would produce an additional exception (since Graal SDK is unable to enumerate its languages). The disable aimed to prevent that and log Graal provider's failure just once (Graal's EngineProvider is a singleton in Lookup, so the same instance is called all the time).

Is it desirable to log each time Scripting.newBuilder().build() is called ?

[matthiasblaesing]

I meant, that we should log the exception here, as it is not reported anywhere else and relevant it a user for example complains about missing PAC support.

The disable part was only meant to say, that from my POV a plain log without an additional guard would be ok, because the exception code path will only be hit once. In Scripting.java you added faultyLookupIds to prevent duplicate reports and that is not necessary here.

[sdedic]

the exception code path will only be hit once.

That's not true IMHO; ServerConfigError will be fired just once, but LinkageError would be fired for every Scripting.newBuilder().build() call without the guard - this provider is bound to fail all the time after ServerConfigurationError condition (and most possibly after a LinkageError originating elsewhere as well). At least if I interpret/debugged the code correctly.

The question is still whether it is desirable to have an exception logged for each Scripting.newBuilder().build() or not.

[matthiasblaesing]

My understanding is, that GraalEnginesProvider.java is loaded as a singleton and thus the second call to factories will not call into enumerateLanguages because disable is already set.

[mbien]

yeah this looks like the catch block can be only reached once since it writes to disable and nothing is ever resetting it -> logging here would be no problem

[sdedic]

@mbien & @matthiasblaesing : finally understood. thanks.

[sdedic]

Logging addressed in 0389c34 (rebased & replaced commits).

[matthiasblaesing]

Looks sane to me. Thank you!