i18n and exception handling

[ataylor]

This I still need to do but there a a couple of ways, especially around teh HornetQException class, since a code is nearly always added it needs to be passed as a property type in the method, for instance:

@Message(id = 119004, value = "Connected server is not a backup server", format = Message.Format.MESSAGE_FORMAT)

   HornetQException notABackupServer(@Property Integer code);

When the logger is generated it will call setCode(..) on HornetQException to do this and would be called like so:

throw HornetQMessageBundle.MESSAGES.notABackupServer(HornetQException.ILLEGAL_STATE);

This is the simplest way but would require passing the code each time, an alternative would be to subclass each type of Exception, for instance:

public class IllegalStateException extends HornetQException

{

   public IllegalStateException(String msg)

   {

      super(HornetQException.ILLEGAL_STATE, msg);

   }

}

and then the logger call would be much simpler:

@Message(id = 119005, value = "Backup replication server is already connected to another server", format = Message.Format.MESSAGE_FORMAT)

   IllegalStateException backupServerAlreadyConnectingToLive();

and called by

throw HornetQMessageBundle.MESSAGES.backupServerAlreadyConnectingToLive();

lastly, we could just internationalise the messages themselves.

I prefer 1 or 2 however I am just thinking, would it be advantagous to sub class the Exceptions so we can get finer controlled handling elsewhere in the code?

thoughts guys?

[clebert.suconic]

I prefer the subClass. It seems cleaner.

I don't think that would cause any issues with the JMS layer since the hierarchy still the same here.

[ataylor]

i agree, if no one has any objections I will start sub classing HornetException

[ataylor]

from email conv:

I was just wondering how to cope with the native exceptions class, Im adding sub classes so need to create a c++ class for each type. If you could point me at where the conversion is done between c and java or just give me a few hints.

Also, because i am refactoring the API, i.e. HornetQ exception, it wont be backward compatible but this is ok yeah? as the next version is a major version release 3.0

[ataylor]

cleberts reply:

Old client needs to be backward compatible. I don't see an issue on the API change here. We will need old clients to talk to new servers. Also I don't think that class will be serialized over the wire.

The AIOException class that you see on the native code is a pure C++ exception and it's not exposed towards the Java layer.

There aren't many exceptions though. Most of the time errors will be sent through callbacks on the AIOCallback. However during initialization states we may eventually see exceptions (like not support file, can't open file, things more brutal)

There's a function on JavaUtilities.cpp. It's doing a findClass and setting the method directly.

I can replace that by a callback on AsyncFileImpl, so the creation will be done in java instead. It would be a simple change and I can do it as soon as I'm done with this patch (and have finished merging my stuff). i.e. early next week.

Here's the function from JavaUtilities.cpp

void throwException(JNIEnv * env, const int code, const char * message)

{

  jclass exceptionClass = env->FindClass("org/hornetq/api/core/HornetQException");

  if (exceptionClass==NULL)

  {

     std::cerr << "Couldn't throw exception message:= " << message << "\n";

     throwRuntimeException (env, "Can't find Exception class");

     return;

  }

  jmethodID constructor = env->GetMethodID(exceptionClass, "<init>", "(ILjava/lang/String;)V");

  if (constructor == NULL)

  {

       std::cerr << "Couldn't find the constructor ***";

       throwRuntimeException (env, "Can't find Constructor for Exception");

       return;

  }

  jstring strError = env->NewStringUTF(message);

  jthrowable ex = (jthrowable)env->NewObject(exceptionClass, constructor, code, strError);

  env->Throw(ex);

}

[ataylor]

Old client needs to be backward compatible. I don't see an issue on the API change here. We will need old clients to talk to new servers. Also I don't think that class will be serialized over the wire.

HornetQException is serializable and does get serialized to the client so when I remove all the error type codes I this will break backward compatibility, however i thought that between major versions we didnt have to worry about this? I was actually going to remove the type and replace with an enum like so:

public enum HornetQExceptionType

{

     INTERNAL_ERROR(000).....

     int type;

     HornetQExceptionType(int type)

     {

          this.type = type

     }

}

This would only be used as a way of tracking the codes we use tho and in tests to check for exception types, rather than changing all the tests to catch specific exceptions.

I can replace that by a callback on AsyncFileImpl, so the creation will be done in java instead. It would be a simple change and I can do it as soon as I'm done with this patch (and have finished merging my stuff). i.e. early next week.

Ok, I will add the appropriate classes and we can change the native layer later

[clebert.suconic]

These are the exceptions I may throw from Native:

#define NATIVE_ERROR_INTERNAL 200

#define NATIVE_ERROR_INVALID_BUFFER 201

#define NATIVE_ERROR_NOT_ALIGNED 202

#define NATIVE_ERROR_CANT_INITIALIZE_AIO 203

#define NATIVE_ERROR_CANT_RELEASE_AIO 204

#define NATIVE_ERROR_CANT_OPEN_CLOSE_FILE 205

#define NATIVE_ERROR_CANT_ALLOCATE_QUEUE 206

#define NATIVE_ERROR_PREALLOCATE_FILE 208

#define NATIVE_ERROR_ALLOCATE_MEMORY 209

#define NATIVE_ERROR_IO 006

#define NATIVE_ERROR_AIO_FULL 211

they match to what's defined on HornetQException

201 and 202 are basically the same.. just a slight variation

203 and 204 are also similar

205 is a native error from AIO...

206 - it's a native error from libaio

208 - that could be considered an IO exception. the disk is full when prefilling a file.

209 - Out Of Memory on native layer

006 - simple IO error

211 - this shouldn't happen on our case as we safeguard the libaio with a semaphore on the java layer. but the native code is also testing for it. it could be exchanged by a general native error.

[borges]

Hi,

Just some comments (on what we were discussing at IRC).

There is one very good argument for having sub-types: it makes for proper exception handling in Java, and as HornetQ has checked exceptions, I guess users are supposed to handle them. There is one minor problem with sub-types which is that they create a lot of boiler-plate code.

I was initially against the sub-types, and for some "umbrela" sub-types which would work with different codes (e.g. HornetQNativeException). Andy made a good argument by asking whether users would need to differentiate these (same exception class, different exception code value) in their code. I guess the answer to that is that we should not have different code values for exceptions which do not need to be differentiated in error handling code.

So vote for adding the sub-classes with an optional reduction in the number codes we have (we should really only have codes that users need to write error handling against, anything more than that can be infered from the stack trace or message).

[ataylor]

here is what i will do, I will finish up the work tomorrow and send a PR, you can both review this (without pulling) and I will make any changes suggested