vaadin / kubernetes-kit

Other
3 stars 3 forks source link

mechanism to detect serialization issues in a running application #108

Closed jorgheymans closed 7 months ago

jorgheymans commented 8 months ago

At several points in time, unserializable objects were introduced in our Vaadin screens unknowingly. We would like to have a mechanism where we can detect such serialization errors in the CI pipeline.

Screen can stop working correctly in a very subtle manner, and if you don't know its a session serde problem you're in for a wild goose hunt to find or even detect the problem. When a session serialization error occurs, maybe you could store info about this in the ServletContext that an application could go and read? Or any other application scope data attribute, does not have to be servlet related even.

Legioth commented 8 months ago

Sounds like exactly what vaadin.devmode.sessionSerialization.enabled is supposed to do. Is there something missing from that approach?

jorgheymans commented 8 months ago

As the doc says:

The results from the test are saved in the server logs. They include test process outcomes (e.g., SERIALIZATION_FAILED, DESERIALIZATION_FAILED, SUCCESS, etc.). They also include a list of unserializable classes, object class graph in case of deserialization errors, and potential causes of SerializedLambda ClassCastException.

The only way to spot them is to investigate the server logs which is difficult to do at runtime (Splunk etc). Given that session serialization errors most likely break the application logic, I was hoping for a better way to detect them when our integration test suite runs.

mcollovati commented 8 months ago

If you have PUSH enabled, you should see a yellow notification box on the page

 The RequestHandler evaluates if the HTTP request produces UI changes
and prepares the tester execution and the result notification on the UI notification,
if server push is enabled
jorgheymans commented 8 months ago

Thanks, but we don't have push enabled. I'm hesitant to enable it for testing only as we would not be testing the application like it is deployed in the production environment.

Legioth commented 8 months ago

Would it make sense to make the error reporting of vaadin.devmode.sessionSerialization.enabled pluggable so that it could report errors in a way that is more applicable for automatic testing?

I'm not exactly sure what kind of reporting would be more appropriate since the errors are detected on the server JVM which is typically separate from the JVM that runs the tests. In addition to requiring some kind of IPC approach, the other implication of this is that the tests would somehow need to wait until there's been enough time to report any errors from the last interactions in each test.

jorgheymans commented 8 months ago

At the very minimum, being able to know more or less directly there's a serde error during interaction vital. If the session serde happens synchronous in the request (?) then a hard fail of said request would be enough. This would not even be related to the session serde debug tool i guess.

Legioth commented 8 months ago

Immediate feedback is challenging from an architectural perspective since the session can be serialized only after the response to the client has been created. I guess it might be possible to first create the response, then serialize the session and only send the response to the client after serialization but this would require some low-level changes in Flow to have a chance of running the serialization logic at exactly the right moment.

jorgheymans commented 8 months ago

Mmm ok i see. Then maybe just as a start you could provide a configurable callback that executes when serde errors occur? That way we can craft something on our side that raises visibility of such errors.

heruan commented 8 months ago

Would this be a good spot to inject a pluggable callback?

https://github.com/vaadin/kubernetes-kit/blob/35f551f9f71ca2a580fbd780f71a34b2f3775119/kubernetes-kit-starter/src/main/java/com/vaadin/kubernetes/starter/sessiontracker/serialization/debug/SerializationDebugRequestHandler.java#L130-L134

The callback could default to the push notifier, but be customizable to anything else.

Legioth commented 8 months ago

Makes sense. The other half is how it would be configured. Is it enough to just register a bean of a specific type with Spring and then let the kit automatically use that bean if it's available?

jorgheymans commented 8 months ago

Would that callback require devtools to be running? My idea was to just have a callback when actual session serialization errors occur, so we don't need to have devtools active as it slows down everything rather a lot.

Something like:

  try {
     sessionSerializer.serialize(theVaadinSession);
  } catch (SerializationException e) {
    customCallback.onSerializationError(e);
  }

EDIT to clarify, i don't expect to do session serde in the application, example above was very high level how it would look in vaadin-flow, the callback class is gives the application a chance to react. In my case probably something like ApplicationContext.close() as it makes no sense to continue the integration tests.

jorgheymans commented 7 months ago

Fantastic, many thanks @tamasmak @heruan !