db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dyre Tjeldvoll (JIRA)" <derby-...@db.apache.org>
Subject [jira] Commented: (DERBY-336) The wrong overload of StandardException::newException() is used in some cases
Date Tue, 07 Jun 2005 12:04:41 GMT
    [ http://issues.apache.org/jira/browse/DERBY-336?page=comments#action_12312870 ] 

Dyre Tjeldvoll commented on DERBY-336:
--------------------------------------

This was a little more complicated than I first thought: It looks like there are three different
cases of incorrect, or at least unconventional, use of StandardException.newException(...):

1) newException() is called with and explicit "null" argument. 
Error codes:
SQLState.SERVICE_DIRECTORY_CREATE_ERROR  XBM0H.D=Directory {0} cannot be created.
SQLState.LOG_FULL  XSLA4.D=Cannot write to the log, most likely the log is full.  Please delete
unnecessary files.  It is also possible that the file system is read only, or the disk has
failed, or some other problems with the media.
SQLState.RAWSTORE_ERROR_RENAMING_FILE  XSRS4.S=Error renaming file (during backup) from {0}
to {1}.
SQLState.RAWSTORE_ERROR_COPYING_FILE  XSRS5.S=Error copying file (during backup) from {0}
to {1}.

In all of these cases it should be safe to remove the "null" as it is not used in the message
text.


2) newException(...) is called with Throwable as the LAST argument, and not the SECOND as
it should be.
Error codes:
SQLState.AUTH_INVALID_AUTHORIZATION_PROPERTY  28501=Invalid database authorization property
''{0}={1}''.
SQLState.SERVICE_DIRECTORY_CREATE_ERROR  XBM0H.D=Directory {0} cannot be created.

These two cases are simply a matter of changing the order of the arguments, i.e. move the
Throwable argument.

SQLState.LANG_FILE_ERROR  X0X63.S=Got IOException ''{0}''.

This is a bit more tricky. Clearly the author of the message text intended that a textual
representation of the original exception should be inserted into the message text. But this
error code is typically used like this:

StandardException.newException(SQLState.LANG_FILE_ERROR, ioe.toString(), ioe)

So even if we fix the order of the arguments (making "ioe" the 2nd argument), we have a situation
where "ioe" is both the next exception, and its string representation gets inserted into the
message string. This seems a bit redundant to me, but I'll let the experts decide what to
do... :)

3) newException(...) is called with multiple arguments of type Throwable. 
Error codes:
SQLState.FILE_CONTAINER_EXCEPTION  XSDG3.D=Meta-data for Container {0} could not be accessed

This error code is used as follows:
StandardException.newException(SQLState.FILE_CONTAINER_EXCEPTION, this, ioe)
So the last "ioe" argument appears to be redundant, and should be removed.

SQLState.FILE_CREATE_NO_CLEANUP     XSDF2.S=Exception during creation of file {0} for container,
file could not be removed.  The exeception was: {1}.

Here it seems that the intention has been to associate two different exceptions with the same
StandardExecption:
StandardException.newException(SQLState.FILE_CREATE_NO_CLEANUP, ioe, file, se)
where "ioe" is an io exception, and "se" is a security exception. "ioe" will be the next exception,
and "se"  will be inserted (as string) at {1}. This error code is also used with an explicit
"null" instead of "se".  In this case I guess the explicit "null" is warranted, although not
very elegant...

SQLState.FILE_CANNOT_REMOVE_FILE    XSDF4.S=Exception during remove of file {0} for dropped
container, file could not be removed {1}.

This error code is used in much the same way as the previous one, except that in the case
where there is only one execption this is used both as the next exception and as the message
argument, rather than using an explicit null.
StandardException.newException(SQLState.FILE_CANNOT_REMOVE_FILE, se, file, se);

When used with two excptions this error reports two io exceptions:
StandardException.newException(SQLState.FILE_CANNOT_REMOVE_FILE, ioe2, file, ioe);

Comments? Please... ;)

> The wrong overload of StandardException::newException() is used in some cases
> -----------------------------------------------------------------------------
>
>          Key: DERBY-336
>          URL: http://issues.apache.org/jira/browse/DERBY-336
>      Project: Derby
>         Type: Bug
>  Environment: Any
>     Reporter: Dyre Tjeldvoll
>     Assignee: Dyre Tjeldvoll
>     Priority: Trivial

>
> When looking at DERBY-128 it became clear that the wrong overload of StandardException::newException()
was used when reporting
> SQLState.SERVICE_DIRECTORY_CREATE_ERROR. The message string only takes one parameter
so only one additional parameter (other than Throwable) should be used:
> PersistentServiceImpl.java:676
>                             throw StandardException.newException(SQLState.SERVICE_DIRECTORY_CREATE_ERROR,
>                                                                  serviceDirectory, null);
> // Calls StandardException.newException(String, Object, Object)
> // Should call StandardException.newException(String, Object)? Or StandardException.newException(String,
Throwable, Object)? With the IOException as  
> // Throwable?
> PersistentServiceImpl.java:692
>         throw StandardException.newException(SQLState.SERVICE_DIRECTORY_CREATE_ERROR,
name, t);
> // Calls StandardException.newException(String, Object, Object)
> // Should call StandardException.newException(String, Throwable, Object)?
> BaseDataFileFactory.java:279
>                 throw StandardException.newException( SQLState.SERVICE_DIRECTORY_CREATE_ERROR,
dataDirectory, ioe);
> // Calls StandardException.newException(String, Object, Object)
> // Should call StandardException.newException(String, Throwable, Object)?

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message