[Geoserver-devel] Headaches with service exception reporting

Hi,
http://jira.codehaus.org/browse/GEOS-1155 is leading me down on a path
to hell.

To recap what's going on for those that aren't informed, we where throwing a classcast exception when translating a filter into sql
due to a bug in the filter splitter, and the bug triggered
because one of the attributes used in the filter was inexistent.

Yet, even with that bug fixed, the error reported is still useless:
"null error:Translator error"

This happens because the GML translator report only that message,
and we don't attach in any way the real error messages, which is
in the nested exception:
java.lang.IllegalArgumentException: Property 'full_name_lc' could not be found in gnis_pop

Ok, so, what do we do? I have various options in mind, like none of
them:
* change the GML translator so that exception is built like:
   e = new TransformerException("Translator error" + e.getMessage(),e);

   This solves the issue with the least effort, provided that the
   real cause is reported at the first level. Or we could go and look
   for the most nested exception.
   Yet, what would happen if we did the same treatment to all
   exceptions? The errors messages would go wild I fear

* report the full stack trace, like we do in WFS module, Geoserver
   trunk. Hum, this one is better, yet the message is a bit criptic

* do a middle version, that is, we print only the messages of
   each exception in the chain (that is, a summary of the stack trace).
   In the above case, the output could look like:

   Translation Error
   Property 'full_name_lc' could not be found in gnis_pop

   Seems to be decent enough for a human being to read.

What do you think? Any bright idea?

Oh, besides that, exception reporting in Geoserver is all over the
place. We should have a single service exception class that wraps the exception and applies whatever solution we want... ServiceException
seems the one, but its constructors allows for a wide variety of
behaviours (only the message, message + exception, message + locator,
you name it, and code that builds error messages its own way, and so on)... I'd like to pin this down and have a single consistent
way of reporting service exception in the code.

Cheers
Andrea

Andrea Aime wrote:

Hi,
http://jira.codehaus.org/browse/GEOS-1155 is leading me down on a path
to hell.

To recap what's going on for those that aren't informed, we where
throwing a classcast exception when translating a filter into sql
due to a bug in the filter splitter, and the bug triggered
because one of the attributes used in the filter was inexistent.

Yet, even with that bug fixed, the error reported is still useless:
"null error:Translator error"

This happens because the GML translator report only that message,
and we don't attach in any way the real error messages, which is
in the nested exception:
java.lang.IllegalArgumentException: Property 'full_name_lc' could not be
found in gnis_pop

Ok, so, what do we do? I have various options in mind, like none of
them:
* change the GML translator so that exception is built like:
   e = new TransformerException("Translator error" + e.getMessage(),e);

   This solves the issue with the least effort, provided that the
   real cause is reported at the first level. Or we could go and look
   for the most nested exception.
   Yet, what would happen if we did the same treatment to all
   exceptions? The errors messages would go wild I fear

* report the full stack trace, like we do in WFS module, Geoserver
   trunk. Hum, this one is better, yet the message is a bit criptic

* do a middle version, that is, we print only the messages of
   each exception in the chain (that is, a summary of the stack trace).
   In the above case, the output could look like:

   Translation Error
   Property 'full_name_lc' could not be found in gnis_pop

   Seems to be decent enough for a human being to read.

What do you think? Any bright idea?

I like Chris's suggestion, option 3 by default and option 2 with
versbose turned ont.

Oh, besides that, exception reporting in Geoserver is all over the
place. We should have a single service exception class that wraps the
exception and applies whatever solution we want... ServiceException
seems the one, but its constructors allows for a wide variety of
behaviours (only the message, message + exception, message + locator,
you name it, and code that builds error messages its own way, and so
on)... I'd like to pin this down and have a single consistent
way of reporting service exception in the code.

Unfortunately different exception cases require code, some require both
code + locator, some require neither, hence the many different
constructors of that class. I did my best to clean it up during ows4 as
it used to contain a lot more stuff, like all the serialization logic
and xml writing code. I am open to suggestions on how to further improve
it. I agree that the mass of constructors make it hard to use. We could
possibly reduce the number of constructors to a minimum, say just
message and cause, and then force people to use setter methods for code
and locator.

Cheers
Andrea

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4007,4680ea6c24975409313003!

--
Justin Deoliveira
The Open Planning Project
http://topp.openplans.org

Go Andrea!

I've had this on the list of things I'd like to see sorted since I first started playing with it.

Its nice to have the full stacktrace when using eclipse - since you can just dive into the stack trace, set a breakpoint, rerun the failure and examine the variables.

But the end user should never have to face that kind of thing when they make an error in the query..

We already have a "verbose errors" flag - perhaps we can do option 3 by default, with full stack trace if verbose errors are set?

RA

Andrea Aime wrote:

Hi,
http://jira.codehaus.org/browse/GEOS-1155 is leading me down on a path
to hell.

To recap what's going on for those that aren't informed, we where throwing a classcast exception when translating a filter into sql
due to a bug in the filter splitter, and the bug triggered
because one of the attributes used in the filter was inexistent.

Yet, even with that bug fixed, the error reported is still useless:
"null error:Translator error"

This happens because the GML translator report only that message,
and we don't attach in any way the real error messages, which is
in the nested exception:
java.lang.IllegalArgumentException: Property 'full_name_lc' could not be found in gnis_pop

Ok, so, what do we do? I have various options in mind, like none of
them:
* change the GML translator so that exception is built like:
   e = new TransformerException("Translator error" + e.getMessage(),e);

   This solves the issue with the least effort, provided that the
   real cause is reported at the first level. Or we could go and look
   for the most nested exception.
   Yet, what would happen if we did the same treatment to all
   exceptions? The errors messages would go wild I fear

* report the full stack trace, like we do in WFS module, Geoserver
   trunk. Hum, this one is better, yet the message is a bit criptic

* do a middle version, that is, we print only the messages of
   each exception in the chain (that is, a summary of the stack trace).
   In the above case, the output could look like:

   Translation Error
   Property 'full_name_lc' could not be found in gnis_pop

   Seems to be decent enough for a human being to read.

What do you think? Any bright idea?

Oh, besides that, exception reporting in Geoserver is all over the
place. We should have a single service exception class that wraps the exception and applies whatever solution we want... ServiceException
seems the one, but its constructors allows for a wide variety of
behaviours (only the message, message + exception, message + locator,
you name it, and code that builds error messages its own way, and so on)... I'd like to pin this down and have a single consistent
way of reporting service exception in the code.

Cheers
Andrea

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
  

Yeah, I agree with Rob. Option 3 by default, and then turning verbose exceptions on does #2. This is currently how things work on 1.5.x. If trunk WFS is reporting full stack traces by default that's a (minor) bug - it should pick up from the verbose exceptions option.

I'm also fine with constraining ServiceException to a single way of reporting.

Chris

Rob Atkinson wrote:

Go Andrea!
I've had this on the list of things I'd like to see sorted since I first started playing with it.

Its nice to have the full stacktrace when using eclipse - since you can just dive into the stack trace, set a breakpoint, rerun the failure and examine the variables.

But the end user should never have to face that kind of thing when they make an error in the query..

We already have a "verbose errors" flag - perhaps we can do option 3 by default, with full stack trace if verbose errors are set?

RA

Andrea Aime wrote:

Hi,
http://jira.codehaus.org/browse/GEOS-1155 is leading me down on a path
to hell.

To recap what's going on for those that aren't informed, we where throwing a classcast exception when translating a filter into sql
due to a bug in the filter splitter, and the bug triggered
because one of the attributes used in the filter was inexistent.

Yet, even with that bug fixed, the error reported is still useless:
"null error:Translator error"

This happens because the GML translator report only that message,
and we don't attach in any way the real error messages, which is
in the nested exception:
java.lang.IllegalArgumentException: Property 'full_name_lc' could not be found in gnis_pop

Ok, so, what do we do? I have various options in mind, like none of
them:
* change the GML translator so that exception is built like:
   e = new TransformerException("Translator error" + e.getMessage(),e);

   This solves the issue with the least effort, provided that the
   real cause is reported at the first level. Or we could go and look
   for the most nested exception.
   Yet, what would happen if we did the same treatment to all
   exceptions? The errors messages would go wild I fear

* report the full stack trace, like we do in WFS module, Geoserver
   trunk. Hum, this one is better, yet the message is a bit criptic

* do a middle version, that is, we print only the messages of
   each exception in the chain (that is, a summary of the stack trace).
   In the above case, the output could look like:

   Translation Error
   Property 'full_name_lc' could not be found in gnis_pop

   Seems to be decent enough for a human being to read.

What do you think? Any bright idea?

Oh, besides that, exception reporting in Geoserver is all over the
place. We should have a single service exception class that wraps the exception and applies whatever solution we want... ServiceException
seems the one, but its constructors allows for a wide variety of
behaviours (only the message, message + exception, message + locator,
you name it, and code that builds error messages its own way, and so on)... I'd like to pin this down and have a single consistent
way of reporting service exception in the code.

Cheers
Andrea

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
  
!DSPAM:4005,4680f98a31135332866982!

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

!DSPAM:4005,4680f98a31135332866982!

------------------------------------------------------------------------

_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4005,4680f98a31135332866982!

--
Chris Holmes
The Open Planning Project
http://topp.openplans.org