Fix exceptions with message: unknown exception type; no further information.
#5080
Conversation
DimitrisStaratzis
changed the title
unknown exception type; no further information.unknown exception type; no further information.
| @@ -338,26 +339,22 @@ QueryPlan deserialize_query_plan_response( | |||
|
|
|||
| void serialize_query_plan_request( | |||
| const Config&, Query&, const SerializationType, Buffer&) { | |||
| throw Status_SerializationError( | |||
| "Cannot serialize; serialization not enabled."); | |||
| throw GenericException("Cannot serialize; serialization not enabled."); | |||
There was a problem hiding this comment.
The reason this is a GenericException and not a SerializationStatusException is because the SerializationStatusException class is excluded from the preprocessor when building with serialization off. So we raise a GenericException which is defined in util.h
There was a problem hiding this comment.
We should define class SerializationDisabledException to use in this case.
There was a problem hiding this comment.
I defined QueryPlanSerializationDisabledException so that we are more precise and follow what happens in other classes like Array as you mentioned
There was a problem hiding this comment.
class GenericException should be replaced with something much more specific. See comments.
I'd prefer seeing an alias using SerializationException = SerializationStatusException so that we don't add all those legacy identifiers to the code.
Otherwise looks good.
tiledb/sm/misc/utils.h
Outdated
| @@ -46,6 +46,14 @@ | |||
| namespace tiledb { | |||
| namespace sm { | |||
|
|
|||
| /** Class for generic status exceptions. */ | |||
| class GenericException : public StatusException { | |||
There was a problem hiding this comment.
We don't want to use this. Calling a StatusException generic requires subverting the requirement that we track origin.
tiledb/sm/misc/utils.h
Outdated
| /** Class for generic status exceptions. */ | ||
| class GenericException : public StatusException { | ||
| public: | ||
| explicit GenericException(const std::string& msg) | ||
| : StatusException("Generic", msg) { | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
| /** Class for generic status exceptions. */ | |
| class GenericException : public StatusException { | |
| public: | |
| explicit GenericException(const std::string& msg) | |
| : StatusException("Generic", msg) { | |
| } | |
| }; |
| @@ -2081,26 +2082,22 @@ Status max_buffer_sizes_deserialize( | |||
|
|
|||
| void serialize_load_array_schema_request( | |||
| const Config&, const LoadArraySchemaRequest&, SerializationType, Buffer&) { | |||
| throw Status_SerializationError( | |||
| "Cannot serialize; serialization not enabled."); | |||
| throw GenericException("Cannot serialize; serialization not enabled."); | |||
There was a problem hiding this comment.
For this exception see, for example, class ArraySerializationDisabledException. We should use the analogous technique here.
I'm not sure we need an exception type for each kind of object being serialized. I'd be good with a single SerializationDisabledException with origin serialization.
There was a problem hiding this comment.
I did not want to deviate from the already used techniques so I created all the necessary *DisabledException classes
There was a problem hiding this comment.
I created all the necessary *DisabledException classes
That's fine too.
| @@ -338,26 +339,22 @@ QueryPlan deserialize_query_plan_response( | |||
|
|
|||
| void serialize_query_plan_request( | |||
| const Config&, Query&, const SerializationType, Buffer&) { | |||
| throw Status_SerializationError( | |||
| "Cannot serialize; serialization not enabled."); | |||
| throw GenericException("Cannot serialize; serialization not enabled."); | |||
There was a problem hiding this comment.
We should define class SerializationDisabledException to use in this case.
This PR migrates from
throw Status_*Error()tothrow *Exception(). This was causing some exceptions to be silenced because the previous implementation was not handling them correctly resulting in the display of the generic error message:unknown exception type; no further informationTo achieve this migration I had to create the following Exception classes:
FragmentInfoException,QueryConditionException,ArraySchemaSerializationDisabledException,ArraySchemaSerializationException,ConsolidationSerializationDisabledException,ConsolidationSerializationException,EnumerationSerializationDisabledException,EnumerationSerializationException,QueryPlanSerializationDisabledException,QueryPlanSerializationException[sc-48757]
TYPE: BUG
DESC: Fix exceptions with message:
unknown exception type; no further information.