SQLExceptionTranslators do not reliably translate QueryTimeoutException [SPR-11959]

[spring-projects-issues]

Francisco Lozano opened SPR-11959 and commented

With SQLErrorCodeSQLExceptionTranslator, my code can catch both DuplicateKeyException and DataIntegrityViolationException. This helps to distinguish between a FK constraint violation and a duplicate key, directly and without having to dig more onto the database to find out.

I had to change my use of JdbcTemplate to force SQLExceptionSubclassTranslator, in order to gracefully handle query timeouts. Unfortunately, this had the effect of rendering all my catches of DuplicateKeyException unreachable.

So, by default and without coding anything, as a user of Spring JDBC wrappers I'm stuck between a rock and a hard place.

I guess the workaround is to make a custom SQLExceptionTranslator, but this use-case is not so strange and it would be great if Spring could come with a "smart" SQL exception translator that uses whatever it needs to provide the most accurate exception possible.

I'm not sure now this should be reported a "bug" or as something else... I initially considered SQLErrorCodeSQLExceptionTranslator "buggy" because it is not translating exceptions onto DuplicateKeyException, but I can see in this case it's not straightforward to do it right for everyone.

---

Affects: 4.0.5

Issue Links:

• java.sql.SQLTimeoutException not translated to org.springframework.dao.QueryTimeoutException [SPR-9376] #14012 java.sql.SQLTimeoutException not translated to org.springframework.dao.QueryTimeoutException

Referenced from: commits 4082274

[spring-projects-issues]

Francisco Lozano commented

Workaround for now:

public class SmarterSQLExceptionTranslator extends
		SQLErrorCodeSQLExceptionTranslator {

	@Override
	protected DataAccessException customTranslate(String task, String sql,
			SQLException sqlEx) {
		if (sqlEx instanceof SQLTimeoutException) {
			return new QueryTimeoutException(buildMessage(task, sql, sqlEx),
					sqlEx);
		}
		return super.customTranslate(task, sql, sqlEx);
	}
}

[spring-projects-issues]

Juergen Hoeller commented

Based on your workaround, let's turn this around: SQLErrorCodeSQLExceptionTranslator should be able to handle QueryTimeoutException, even if not indicated by a specific error code.

As for SQLExceptionSubclassTranslator, that one is intended to be constrained to the JDBC 4.0 exception hierarchy's capabilities. It's also used as the fallback translator within SQLErrorCodeSQLExceptionTranslator, but for some reason it's not kicking in for query timeouts - probably because those end up being misidentified based on some error code match in SQLErrorCodeSQLExceptionTranslator itself?

Juergen

[spring-projects-issues]

Francisco Lozano commented

First of all, an update: for me it's still failing even with workaround, because I'm using MySQL and this issue happens: http://bugs.mysql.com/bug.php?id=73216. In short: MySQL has two MySQLTimeoutException - one jdbc4 and other plain SQLException - and the one being thrown is the one which doesn't inherit from SQLTimeoutException.

During my tests I threw manually the right exception (the jdbc4 one), whereas the driver never throws that one (it's there but I think it's never used).

If the thrown MySQLTimeoutException was the correct one of the two that they have in the driver, SQLErrorCodeSQLExceptionTranslator would probably work OK as it is now, right? delegating to subclass-based.

So, unless there's another way to detect a timeout other than subclass-approach, and given that SQLExceptionSubclassTranslator is on-spec and OK as it is (it already does as much as it can with subclass strategy), I think this issue may be fixed as a won't fix.

[spring-projects-issues]

Juergen Hoeller commented

Indeed, SQLErrorCodeSQLExceptionTranslator is supposed to fall back to the subclass-based approach in such a case. The only problem could be that it accidentally detects an known error code in a timeout-related exception and decides to translate it to one of its resource failure exceptions first.

That said, in your case, it seems that it does fall through to the subclass-based approach but just doesn't detect it that way either since it's not the expected subclass. So you end up getting an UncategorizedSQLException? Or do you get a more specific but still not timeout-indicating exception thrown?

We could have a fallback check that detects known keywords such as "Timeout" in an exception's class name before resorting to an UncategorizedSQLException. I suppose that would actually work for this MySQL case here... In principle, we'd even be willing to add such a workaround just for MySQL.

Juergen

[spring-projects-issues]

Francisco Lozano commented

In my case, I get UncategorizedSQLException wrapping the non-jdbc4 MySQLTimeoutException.

I like the idea of having a list of (vendor-specific?) exception names or patterns handling fallbacks. I think I will try that for my workaround actually... I didn't want to catch the specific exception and couple my code to MySQL's driver because of GPL, but I can for sure put the name of the exception in a Set.

[spring-projects-issues]

Juergen Hoeller commented

I'm not too keen on turning this into a full-fledged extensible mechanism at this point, since we only have one use case and everything else is known to be covered by the existing (and extensible) error code and SQLState based mechanisms. So from that perspective, all I'm considering to add is this...

// For MySQL: exception class name indicating a timeout?
// (since MySQL doesn't throw the JDBC 4 SQLTimeoutException)
if (ex.getClass().getName().contains("Timeout")) {
     return new QueryTimeoutException(buildMessage(task, sql, ex), ex);
}

at the end of SQLStateSQLExceptionTranslator which is the last-resort fallback in all of our translation chains, right before we end up with an UncategorizedSQLException.

Juergen

[spring-projects-issues]

Francisco Lozano commented

If this is the only case found, maybe that's enough :) but the .contains("") is not very roubst, why not the class name? if it's just for MySQL's specific case...

[spring-projects-issues]

Juergen Hoeller commented

Fair enough, we could check for the specific class name there... It just might be beneficial to cover similar exceptions for other JDBC drivers as well; in some sense, that's actually more robust. We have a few such cases where we detect naming patterns for the purposes of specific exception messages etc, for cases where there are no formal indications. Note that we are checking the exception class name here, not the exception message.

Since the specific check above is right before giving up completely (with a bland UncategorizedSQLException), a general check for timeout-related exceptions doesn't seem to hurt. It will even cover plain SQLStateSQLExceptionTranslator use with the standard JDBC 4 SQLTimeoutException, without the SQLExceptionSubclassTranslator involved, adding a timeout capability that otherwise isn't possible based on the pure SQLState arrangement.

Juergen

[spring-projects-issues]

Francisco Lozano commented

Thank you as always!