Closed spring-operator closed 15 years ago
spring-operator
commented
15 years ago Keith Donald commented
Does your flow enter a view-state after it handles the exception? That is the expected way to pause the flow, and key assignment happens when a ViewState is entered. If you enter a custom state type you'll be expected to do similar ViewState logic yourself.
spring-operator
commented
15 years ago Keith Donald commented
Improved javadoc around FlowExecutionHandler implementation.
spring-operator
commented
15 years ago Triqui Galletas commented
Well, they key of this issue is that it happens when a exception is thrown BEFORE any viewstate has been entered (ie on flow start, when creating variables, on first viewstate start)
I'm not completely sure after such a long time, but if I remember correctly this also happens with flow transitions on-exception.
The question is that if the exception is thrown BEFORE a viewstate is entered, then the FlowExecutionImpl has the following values: started=true flowSessions.size()=0 key=null So, The flow has started (hasStarted() is true) is not active (isActive() is false), and hence it has ended (hasEnded() is true) before it started. The resulting state of the flow is that it was not fully started (started is true but key is null and flowSessions is empty) and it was paused, but when it tries to resume it throws the above exception or an IllegalStateException (from getActiveSession()).
That's why I switched from transitions on-exception to exception-handler, that way I can add this in my handler: public boolean canHandle(FlowExecutionException ex) { return ex.getStateId() != null && canHandleWithState(ex); }
But that completely kills the goal of sending the flow to a custom state when an exception is thrown. Only if it is thrown BEFORE a viewstate has been entered but still.
Maybe your changes to javadocs explain a solution to this, but I don't know where to check it, how can I have a look at the modified javadocs?
spring-operator
commented
15 years ago Triqui Galletas commented
Ok, you are right, the solution is to trigger a transition to a viewstate after you handle the exception. The error is trying to call requestFlowExecutionRedirect() before a view state has been entered. I had already fixed it with a few conditions. You can check the code below. My mistake was to think that this was the same error as before. The error reported was caused by calling requestFlowExecutionRedirect when no viewstate had been entered. That's a programmer's error. But the error I'm getting is caused by calling context.getCurrentState() to try and find the proper viewstate to transition to. This is a bug, I think.
The first one is the exception posted above, but the second one is this: IllegalStateException: No active session to access; this flow execution has ended.
In my exception-handler, I was trying to go to the previous viewstate with this: public void handle(FlowExecutionException ex, RequestControlContext context) { logException(ex, context); context.getMessageContext().addMessage(buildMessage(ex, context)); // Redirect to originating view state to display message if (context.getCurrentState().isViewState()) { context.getExternalContext().requestFlowExecutionRedirect(); return; } State originatingViewState = (State)context.getRequestScope().get("webflow.originatingViewState"); if (originatingViewState != null) { context.execute(new Transition(new DefaultTargetStateResolver(originatingViewState.getId()))); return; } context.execute(new Transition(new DefaultTargetStateResolver(getExceptionTargetState()))); }
The problem with this is that context.getCurrentState() throws an IllegalStateException. From RequestContext#getCurrentState: Returns the current state of the executing flow. May return null if this flow execution is in the process of starting and has not yet entered its start state. Throws: java.lang.IllegalStateException - if this flow execution has not been started at all, or if this execution has ended and is no longer actively executing
According to this it should return null when handling a exception which happened after flow started and before the start state is entered. This is ambiguous since it will thrown the IllegalStateException in some cases where the flow could be considered as already started.
I've been debugging a little and I think that this will only happen when it throws a security exception (ie AccessDeniedException) or any other exception on a FlowExecutionListener, since those will be thrown before the flowSession is created and thus leaving the flow in a wrong state (described in my previous post).
From FlowExecutionImpl: void start(Flow flow, MutableAttributeMap input, RequestControlContext context) { listeners.fireSessionCreating(context, flow); // <- If a exception is thrown here the problem arises (ie security exceptions) FlowSession session = activateSession(flow); // <- Exceptions thrown after this line will cause no problem if (input == null) { input = new LocalAttributeMap(); } StateManageableMessageContext messageContext = (StateManageableMessageContext) context.getMessageContext(); messageContext.setMessageSource(flow.getApplicationContext()); listeners.fireSessionStarting(context, session, input); flow.start(context, input); listeners.fireSessionStarted(context, session); }
spring-operator
commented
15 years ago Keith Donald commented
/**
A strategy for handling an exception that occurs at runtime during the execution of a flow definition.
Note: special care should be taken when implementing custom flow execution exception handlers. Exception handlers are
like Transitions in that they direct flow control when exceptions occur. They are more complex than Actions, which
can only execute behaviors and return results that drive flow control. For this reason, if implemented incorrectly, a
FlowExecutionHandler can leave a flow execution in an invalid state, which can render the flow execution unusable or
its future use undefined. For example, if an exception thrown at flow startup gets routed to an exception handler,
the handler must take responsibility for ensuring the flow execution returns control to the caller in a consistent
state. Concretely, this means the exception handler must transition the flow to its start state. The handler should
not simply return leaving the flow with no current state set.
Note: Because flow execution handlers are more difficult to implement correctly, consider catching exceptions in your
web flow action code and returning result events that drive standard transitions. Alternatively, consider use of the
existing {@link TransitionExecutingFlowExecutionExceptionHandler} which illustrates the proper way to implement an
exception handler.
@author Keith Donald
*/
public interface FlowExecutionExceptionHandler {
/**
@param exception the exception that occurred@return true if yes, false if no
*/
public boolean canHandle(FlowExecutionException exception);/**
@link RequestControlContext#execute(Transition)}.@param exception the exception that occurred@param context the execution control context for this request
*/
public void handle(FlowExecutionException exception, RequestControlContext context);
}spring-operator
commented
15 years ago Triqui Galletas commented
Well, I know it's a small bug, but it's a bug after all. In fact, the proper way, according to the docs you posted, implemented by TransitionExecutingFlowExceptionHandler does not work that great either. As I said the error will happen only when an exception is thrown by a FlowExecutionListener at the sessionCreating method, and the only exception possible at this point included in Spring out of the box is the AccessDeniedException. Neither HibernateFlowExecutionListener nor JpaFlowExecutionListener throw exceptions at sessionCreating. Don't know about custom listeners from other users or other spring listeners yet to be implemented.
I tried a small example with a secured flow and with a more secured view state, if you have the role ADMIN or GOD everything works fine, but if you enter the flow with any other role, this is what TransitionExecutingFlowExceptionHandler throws:
\<flow xmlns="http://www.springframework.org/schema/webflow" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.springframework.org/schema/webflow http://www.springframework.org/schema/webflow/spring-webflow-2.0.xsd">
\
java.util.NoSuchElementException at java.util.LinkedList.getLast(Unknown Source) at org.springframework.webflow.engine.impl.FlowExecutionImpl.getActiveSessionInternal(FlowExecutionImpl.java:559) at org.springframework.webflow.engine.impl.FlowExecutionImpl.getCurrentState(FlowExecutionImpl.java:652) at org.springframework.webflow.engine.impl.FlowExecutionImpl.execute(FlowExecutionImpl.java:377) at org.springframework.webflow.engine.impl.RequestControlContextImpl.execute(RequestControlContextImpl.java:201) at org.springframework.webflow.engine.support.TransitionExecutingFlowExecutionExceptionHandler.handle(TransitionExecutingFlowExecutionExceptionHandler.java:110)
spring-operator
commented
15 years ago Keith Donald commented
Triqui,
I appreciate your help researching and debugging issue.
I have just committed some changes that should resolve the issue. Specifically, getActiveSession/getCurrentState and co will return NULL if the root session has not been created yet because the flow is still starting. Those methods should only throw exceptions now when the flow is NOT_STARTED or has ENDED. I have introduced a FlowExecutionStatus enum that we use to track statuses more clearly internally now. This has been committed in trunk. Is it possible you can run your tests against these fixes and report back? We're close to releasing 2.0.6 so if you have the opportunity to try it out today that'd be great.
Thanks
spring-operator
commented
15 years ago Keith Donald commented
See Committed revision 2282.
spring-operator
commented
15 years ago Triqui Galletas commented
Still same error. Same exception as before.
I like the new approach, but if you check the last exception I posted in FlowExecutionImpl the method getCurrentState needs to be change as well.
You changed this: public void setCurrentState(String stateId) { FlowSessionImpl session; if (status == FlowExecutionStatus.NOT_STARTED) { session = activateSession(flow); status = FlowExecutionStatus.STARTED; } else { session = getActiveSessionInternal(); } State state = session.getFlow().getStateInstance(stateId); session.setCurrentState(state); }
And you should do the same here to validate the FlowExecutionStatus as well: private State getCurrentState() { FlowSessionImpl session = getActiveSessionInternal(); State currentState = (State) session.getState(); return currentState; }
I'm not completely sure what activateSession(...) does, so I won't dare to suggest the new code.
I'm sorry but we are close to release date too, and I'm afraid I don't have much time to test this a lot. Specially since I have a workaround with a subflow-state which handles secured subflows and is inherited by all subflow-states in the application. Regards.
spring-operator
commented
15 years ago Keith Donald commented
Are you sure you tested against revision 2282 or greater? The code you pasted in is not 2282 or >. I'm not asking for a lot of testing, just testing against 2282 or >.
spring-operator
commented
15 years ago Triqui Galletas commented
Sorry. When I checked the repository the last revision was 2281, maybe it didn't refresh properly. The new revision looks a lot better!! I'm checking it again right now.... and.... oh... Maybe I'm missing something again, but I got this exception: java.lang.NullPointerException at org.springframework.webflow.engine.support.DefaultTargetStateResolver.resolveTargetState(DefaultTargetStateResolver.java:60) at org.springframework.webflow.engine.Transition.execute(Transition.java:217) at org.springframework.webflow.engine.impl.FlowExecutionImpl.execute(FlowExecutionImpl.java:375) at org.springframework.webflow.engine.impl.RequestControlContextImpl.execute(RequestControlContextImpl.java:206)
This is the debug:
public State resolveTargetState(Transition transition, State sourceState, RequestContext context) {
String targetStateId = (String) targetStateIdExpression.getValue(context);
if (targetStateId != null) {
return ((Flow) context.getActiveFlow()).getStateInstance(targetStateId); // <------------------------ throws NullPointerException
} else {
return null;
}
}
public FlowDefinition getActiveFlow() {
FlowSession session = flowExecution.getActiveSession(); // <------------------------returns null
return session != null ? session.getDefinition() : null;
}
public FlowSession getActiveSession() {
if (status == FlowExecutionStatus.NOT_STARTED) {
throw new IllegalStateException("No active session to access; this flow execution has not been started");
}
if (status == FlowExecutionStatus.ENDED) {
throw new IllegalStateException("No active session to access; this flow execution has ended");
}
return getActiveSessionInternal(); // <------------------------returns null
}
private FlowSessionImpl getActiveSessionInternal() {
if (flowSessions.isEmpty()) {
return null; // <------------------------returns null
}
return (FlowSessionImpl) flowSessions.getLast();
}
Triqui Galletas opened SWF-1004 and commented
see http://forum.springsource.org/showthread.php?p=218852
I have an evaluate expression on-start. If it fires an exception it is catched by my exceptionhandler and converted to a message to be displayed to the user. But the problem is that, as the flow continues (given that the exception is already handled), when it tries to call putFlowExecution(...) it fires another exception because flow has not been fully started yet.
SEVERE: Servlet.service() for servlet Spring MVC Dispatcher Servlet threw exception java.lang.IllegalStateException: The key for the flow execution is null; make sure the key is assigned first. Execution Details = [FlowExecutionImpl@1c187c8 flow = 'lookupCustomer', flowSessions = list[[FlowSessionImpl@b95f72 flow = 'lookupCustomer', state = 'lookupCustomer', scope = map[[empty]]]]] at org.springframework.webflow.execution.repository.support.AbstractFlowExecutionRepository.assertKeySet(AbstractFlowExecutionRepository.java:189) at org.springframework.webflow.execution.repository.impl.DefaultFlowExecutionRepository.putFlowExecution(DefaultFlowExecutionRepository.java:108) at org.springframework.webflow.executor.FlowExecutorImpl.launchExecution(FlowExecutorImpl.java:142) at org.springframework.webflow.mvc.servlet.FlowHandlerAdapter.handle(FlowHandlerAdapter.java:183) at org.springframework.webflow.mvc.servlet.FlowController.handleRequest(FlowController.java:172) at org.springframework.web.servlet.mvc.SimpleControllerHandlerAdapter.handle(SimpleControllerHandlerAdapter.java:48) at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:875) at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:809) at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:571) at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:501) at javax.servlet.http.HttpServlet.service(HttpServlet.java:617) at javax.servlet.http.HttpServlet.service(HttpServlet.java:717) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:290) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233) at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191) at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:128) at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:102) at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:109) at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:286) at org.apache.coyote.http11.Http11Processor.process(Http11Processor.java:845) at org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.process(Http11Protocol.java:583) at org.apache.tomcat.util.net.JIoEndpoint$Worker.run(JIoEndpoint.java:447) at java.lang.Thread.run(Thread.java:619)
Affects: 2.0.5