Skip to content
This repository has been archived by the owner on Feb 7, 2018. It is now read-only.

TomcatSessionConverterChain toSession(DynamoSessionItem sessionItem) - stop suppressing exception #29

Closed
AlexKovalevich opened this issue Aug 25, 2015 · 2 comments

Comments

@AlexKovalevich
Copy link

The method iterates throw available tomcatSessionCoverters and if none is able to deserialize dynamodb it throws SessionConversionException supression the real cause. Now imaging this happens in production and there is no way to tell what is the reason.

Method must NOT consume original SessionConversionException.

A quick possible solution is if we reach that point where
"throw new SessionConversionException("Unable to convert Dynamo storage representation to a Tomcat Session with any converter provided");" then:

  1. option: we add a collection of previous exceptions generated by converter.toSession(sessionItem) so administrator/dev can analyze what the issue is.

  2. Not re throw, but add a logger into TomcatSessionConverterChain

  3. Throw Catch clause should NOT be used for expected normal flow control. if-else for, while etc should. This is a small design flaw to me. converter.toSession() should understand know if it can handle object of this type. Usually similar ideas are implemented by isSupported method. This may be more labor intensive to do as it will require some kind of marker how DynamoDBSession was created and what it's type. SessionConversionException should be thrown ONLY IF: a) supported converter has failed. b) no supported converter was found.

  4. I think it is very important for some cases: add a configuration option to enforce recreation of new session object if the old one can not be deserialized. It's quite common case that someone can forget to add serialVersionUID and object can not be restored after recompilation. This flag would at least allow to tolerate this until next tomcat restart.

Thank you.

DynamoSessionItemConverter

@shorea
Copy link
Contributor

shorea commented Aug 25, 2015

I think enough time has passed to get rid of the Converter Chain completely. It was added to ease transition from 1.x versions that persisted sessions in a different format then 2.x versions.

@shorea
Copy link
Contributor

shorea commented Sep 25, 2015

@shorea shorea closed this as completed Sep 25, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants