Skip to content

JedisUtils shouldn't wrap unknow Exception [DATAREDIS-82] #622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
spring-projects-issues opened this issue Mar 5, 2012 · 11 comments
Closed
Labels
type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link

Thomas Bruyelle opened DATAREDIS-82 and commented

The project I'm working on has 2 data access configuration, one with Spring-data-jpa, and an other with Spring-data-redis.

Since I added the JedisConnectionFactory as a Spring bean, one of my JPA-based junit failed because it attempts to catch a DataIntegrityViolationException. But instead this exception is wrapped into a JedisSystemException (see JedisUtils.convertJedisAccessException()).

When I read the code in ChainedPersistenceExceptionTranslator, I understand that a PersistenceExceptionTranslator that doesn't handle an exception should return null. So I think JedisUtils.convertJedisAccessException() should return null also in case of unhandled exception


Affects: 1.0.0 GA

Referenced from: commits bb9418b

1 votes, 2 watchers

@spring-projects-issues
Copy link
Author

Thomas Bruyelle commented

If my understanding is correct, please find this pull request that fix this issue.
#2

@spring-projects-issues
Copy link
Author

Kyrill Alyoshin commented

Yes, this issue is a high-priority one. It basically breaks ANY spring application that has normal JDBC based @Repository classes. Any exception thrown from @Repositories is now getting converted into a RedisSystemException. :-)

Big oversight. We basically have to resort to having a local fork of spring-data-redis

@spring-projects-issues
Copy link
Author

Kyrill Alyoshin commented

Clear violation of PersistenceExceptionTranslator interface.

Verbatim from javadoc:

Do not translate exceptions that are not understand by this translator
for example, if coming from another persistence framework, or resulting
from user code and unrelated to persistence.

@spring-projects-issues
Copy link
Author

Thomas Bruyelle commented

Hey folks, can somebody fix this issue ?

It's almost critical and won't take a long time to fix !

@spring-projects-issues
Copy link
Author

Costin Leau commented

Hi guys,

Sorry for the delay guys - PTO and travel resulted in lack of internet. I'll look at the issue and apply a fix in the next couple of days.
The reason the translator handles raw Exceptions is because Jedis itself throws such exceptions - potentially we can filter this to avoid being way to generic

@spring-projects-issues
Copy link
Author

Kyrill Alyoshin commented

I'd say just don't translate those Redis exceptions that don't inherit base Redis exception.

@spring-projects-issues
Copy link
Author

Costin Leau commented

That's the problem - in Jedis, there is no "base Redis exception"

@spring-projects-issues
Copy link
Author

Costin Leau commented

Hi guys,

This is now finally fixed in master and will be part of the upcoming 1.0.1. Please give the snapshot [1] a try and report back.

[1] https://build.springsource.org/browse/SPRINGDATA-DATAREDISNIGHTLY-280

@spring-projects-issues
Copy link
Author

Christopher Exell commented

Any updates on the release timing for 1.01? I am also affected by this issue.

I'm happy to try the build snapshot if someone can get it into the maven snapshot repository

@spring-projects-issues
Copy link
Author

Costin Leau commented

Have you tried the maven repo listed on the project homepage? http://www.springsource.org/spring-data/redis#maven
It contains the nightly build, which includes the fix for this issue

@spring-projects-issues
Copy link
Author

Christopher Exell commented

The build snapshot mentioned on the project home page is 1.0.0-BUILD-SNAPSHOT, once I moved to 1.0.1.BUILD-SNAPSHOT, I picked up the changes. The changes do resolve the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

1 participant