[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #292: Adds serializability checks for UDFs

[GitBox <gi...@apache.org>]

mbeckerle commented on a change in pull request #292: Adds serializability checks for UDFs
URL: https://github.com/apache/incubator-daffodil/pull/292#discussion_r348774215
 
 

 ##########
 File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/udf/UserDefinedFunctionService.scala
 ##########
 @@ -226,13 +228,35 @@ object UserDefinedFunctionService extends Logging {
           val maybeUdf =
             try {
               val udf = udfInfo.provider.createUserDefinedFunction(namespaceURI, fname)
-              Option(udf)
+              /*
+               * This is to check for any errors thrown when if we try to serialize the
+               * UDF, such as when using save-parser
+               */
+              try {
+                new ObjectOutputStream(new ByteArrayOutputStream()).writeObject(udf)
+                Option(udf)
+              } catch {
+                case e: NotSerializableException =>
+                  log(LogLevel.Error, "Error serializing initialized User Defined Function: %s. Could not serialize member of class: %s",
+                    udf.getClass.getName, e.getMessage)
+                  None
+              }
             } catch {
+              /*
+             * This is to protect against any errors thrown when we are trying to
+             * initialize the UDFs. It will catch any exceptions and emit them as the reason the UDF
+             * is unsupported
+             */
               case e: ReflectiveOperationException => {
                 log(LogLevel.Error, "Error initializing User Defined Function: %s. Error thrown: %s",
                   udfid, e.getCause)
                 None
               }
+              case e: Exception => {
 
 Review comment:
   Let's say the serialization code for a UDF throws a divide-by-zero arithmetic exception. 
   
   What do we want to happen? I think we want a Daffodil UDF-oriented exception object with a cause that is the arithmetic exception.  I believe we should not just log this. We need to let the exception propagate. 
   
   The "this is a bug" banner is appropriate for this situation. Some of these "User" defined functions will be ours. Their failure really would be a bug. 
   
   Usually, if you are catching very general things like Exception, you can repackage things into other exception objects, but you should end by re-throwing something. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services