camunda / camunda-bpm-platform

Flexible framework for workflow and decision automation with BPMN and DMN. Integration with Quarkus, Spring, Spring Boot, CDI.
https://camunda.com/
Apache License 2.0
4.11k stars 1.55k forks source link

Script Task (using GraalJS) and request scoped Spring beans #2794

Closed ThorbenLindhauer closed 1 year ago

ThorbenLindhauer commented 2 years ago

This issue was imported from JIRA:

Field Value
JIRA Link CAM-14852
Reporter VQ1WLag
What is this name? This pseudonym name was generated based on the user name in JIRA to protect the personal data of our JIRA users. You can use this identifier to search for issues by the same reporter.
Has restricted visibility comments true

Environment (Required on creation):

Spring Boot and Camunda Spring Boot Starter.

Description (Required on creation; please attach any relevant screenshots, stacktraces, log files, etc. to the ticket):

A process that has a Timer (or anything async) that ends up executing a Script Task with Javascript using GraalJS as part of the job, while having a Spring Bean that is request scoped, will throw the following exception:

Caused by: java.lang.IllegalStateException: No thread-bound request found: Are you referring to request attributes outside of an actual web request, or processing a request outside of the originally receiving thread? If you are actually operating within a web request and still receive this message, your code is probably running outside of DispatcherServlet: In this case, use RequestContextListener or RequestContextFilter to expose the current request.
    at org.springframework.web.context.request.RequestContextHolder.currentRequestAttributes(RequestContextHolder.java:131) <sub><spring-web-5.3.18.jar:5.3.18>
    at org.springframework.web.context.request.AbstractRequestAttributesScope.get(AbstractRequestAttributesScope.java:42) </sub><spring-web-5.3.18.jar:5.3.18>
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:371) ~<spring-beans-5.3.18.jar:5.3.18>
    ... 99 common frames omitted 

As this is ran in a job that is being processed by Camunda, this thread is not web related, so any Request scoped beans won't be accessible.

Steps to reproduce (Required on creation):

  1. Create a Spring Boot project with Camunda Spring Boot starter
  2. Add attached sample.bpmn 
  3. Create a random bean that has @RequestScope
  4. Start process instance and wait till timer picks up job, exception above will be thrown.

Observed Behavior (Required on creation):

The Camunda Spring Boot starter adds a SpringBeansResolverFactory which will add all beans in the application context as a context for all scripts to be used.

When executing javascript with GraalJS, Camunda will eventually call evaluate() on the ScriptEngine, which in this case is GraalJS and passes along the ScriptBindings (Camunda implementation of Bindings) to GraalJSScriptEngine which will eventually call getOrCreateGraalJSBindings. This method basically moves the Camunda Script Bindings to an internal object of GraalJS, the way they do it is using putAll(). This putAll uses the entrySet() method, which is implemented on the Camunda's ScriptBindings which ends up calling calculateBindingMap() method.

Now, if you have a process instance that ends up executing a Script Task as part of a Job (timer, async before, etc) AND have a Spring bean set with @RequestScope, it will throw the above mentioned exception. Which in this case is happening because of calculateBindingMap() is retrieving the value of all beans.

Expected behavior (Required on creation):

The SpringBeansResolverFactory should only include all beans that are of scope Singleton.

Root Cause (Required on prioritization):

The Root Cause for this is the SpringBeansResolverFactory, as its adding all beans regardless of scope to the bindings for the script engine.

Solution Ideas (Optional):

Hints (optional):

Links:

ThorbenLindhauer commented 2 years ago

This comment was imported from JIRA and written by user @tasso94


Hi VQ1WLag,

Thank you for reaching out to us with your problem.

I investigate bug reports brought up by the community on a fortnightly basis. The next round is scheduled for Friday, the 9th of September.

Stay tuned!

Best, Tassilo

ThorbenLindhauer commented 2 years ago

This comment was imported from JIRA and written by user @tasso94


Hi VQ1WLag,

Due to other responsibilities, I have to bump my investigations by one week.

Stay tuned!

Best, Tassilo

ThorbenLindhauer commented 2 years ago

This comment was imported from JIRA and written by user @tasso94


Hi VQ1WLag,

I have had a look at your problem and understand it so far. Thank you for the detailed description.

However, I wasn't able to reproduce your problem. I have created a simple spring boot project according to your steps to reproduce. You can find it here: <^CAM-14852.tar>

I'm using the following JDK build:

openjdk version "17.0.2" 2022-01-18
OpenJDK Runtime Environment Temurin-17.0.2<ins>8 (build 17.0.2</ins>8)
OpenJDK 64-Bit Server VM Temurin-17.0.2<ins>8 (build 17.0.2</ins>8, mixed mode)

Please check it out and let me know what you are doing differently.

Thank you for your attention and participation.

Best, Tassilo

ThorbenLindhauer commented 2 years ago

This comment was imported from JIRA and written by user VQ1WLag

What is this name? This pseudonym name was generated based on the user name in JIRA to protect the personal data of our JIRA users. You can use this identifier to search for issues by the same reporter.


Hi @tassilo.weidner,

Thanks for checking it out. I had a quick look at your code. The RequestScoped class also needs to include a @Component annotation to make it an actual bean, just @RequestScoped is not enough.

I added it in this attachment and you'll see the error. (Additionally on startup it starts a process instance now, so it will happen instantly) See the attachment here: <^Archive.zip>

So to be clear, the issue you will observe is that, even when not using the RequestScoped bean, just simply having it registered in the app context will throw this exception.

Thanks!

ThorbenLindhauer commented 2 years ago

This comment was imported from JIRA and written by user @tasso94


Hi VQ1WLag,

Thanks! I can reproduce it now. :)(y)

Instead of not registering non-singleton beans, we could catch the exception, return null and write a log message. There might be scenarios where users want to access request-scoped beans: e.g., starting a process instance that executes a script task within a custom REST API endpoint.

What do you think about it?

Best, Tassilo

ThorbenLindhauer commented 2 years ago

This comment was imported from JIRA and written by user VQ1WLag

What is this name? This pseudonym name was generated based on the user name in JIRA to protect the personal data of our JIRA users. You can use this identifier to search for issues by the same reporter.


Hi Tassilo Weidner, 

Sorry for the late response! 

I think logging a message is fine. Although, this should specifically only be done when there is no web related thread, otherwise it should work (as you mentioned yourself).

Best,

Niels

 

ThorbenLindhauer commented 2 years ago

This comment was imported from JIRA and written by user @tasso94


Hi VQ1WLag,

Thank you for your response.

I created a fix: https://github.com/camunda/camunda-bpm-platform/pull/2023

I'll wait for CI feedback and assign it afterward for review.

Right now, we have Code Freeze until 7.18.0 is released. I'll merge the fix as soon as the Code Freeze is over, and you can most likely try it out with 7.19.0-alpha1.

Stay tuned!

Best, Tassilo