controller action exception handlers and command object handling

[Jeff Scott Brown]

I spent some time today working on GRAILS-11576 and have come up with an idea that I would like some feedback on.

When you write a controller like this:

class SomeController {
    def someAction(SomeCommandObject sco) {
        // your code...
    }
}

Grails compiles that to something like this:

class SomeController {
    def someAction() {
        def co = initializeCommandObject(SomeCommandObject)
        // parse the body, do data binding, do DI

        someAction(co)
    }

    def someAction(SomeCommandObject sco) {
        try {
            // your code...
        } catch (Exception e) {
            // check to see if there is a corresponding
            // exception handling method.  if there is
            // call it, otherwise throw e
        }
    }

    def initializeCommandObject(Class type) {
        def obj = type.newInstance()
        // if the request has a body, parse it and
        // do data binding with it, otherwise do
        // data binding with params

        obj
    }
}

(There is other stuff, I am only including the bits relevant to this discussion)

I made a change today to do something like this:

class SomeController {
    def someAction() {
        def co = initializeCommandObject(SomeCommandObject)
        // parse the body, do data binding, do DI

        someAction(co)
    }

    def someAction(SomeCommandObject sco) {
        try {
            // your code...
        } catch (Exception e) {
            // check to see if there is a corresponding
            // exception handling method.  if there is
            // call it, otherwise throw e
        }
    }

    def initializeCommandObject(Class type) {
        def obj = type.newInstance()
        try {
        // if the request has a body, parse it and
        // do data binding with it, otherwise do
        // data binding with params
        } catch (DataBindingSourceCreationException e) {
            obj.errors.addError(new ObjectError(…))
        }

        obj
    }
}

With that change, if a DataBindingSourceCreationException occurs (for example, if malformed JSON or XML are in the body) then instead of an exception being thrown, an error is added to the command object and the controller action is still invoked.

I think that is useful and an improvement over what we had previously.

Tonight I worked up a small addition to that, which does something like this…

class SomeController {
    def someAction() {
        try {
            def co = initializeCommandObject(SomeCommandObject)
            // parse the body, do data binding, do DI

            someAction(co)
        } catch (Exception e) {
            // check to see if there is a corresponding
            // exception handling method.  if there is
            // call it, otherwise throw e
        }
    }

    def someAction(SomeCommandObject sco) {
        // your code...
    }

    def initializeCommandObject(Class type) {
        def obj = type.newInstance()
        try {
       // if the request has a body, parse it and
       // do data binding with it, otherwise do
       // data binding with params
        } catch (DataBindingSourceCreationException e) {
            // maybe should catch java.lang.Exception instead of narrowing
            // to DataBindingSourceCreationException
            if(controllerHasExceptionHandlerMethodForDataBindingSourceCreationException) {
                throw e
            }
            obj.errors.addError(new ObjectError(…))
        }

        obj
    }
}

There are 2 changes there to note.  One is that the try/catch that was inside of the someAction(SomeCommandObject) method has been moved up to the no-arg version of the method.  The other is that the initializeCommandObject(Class) method now checks to see if an exception handler method exists that is compatible with DataBindingSourceCreationException.  If one does exist, the exception is thrown and will eventually be passed to that method instead of invoking the controller action.  If one does not exist, then fall back to populating the command object with an error and invoking the action.  That will allow applications to do something like this:

class SomeController {
    def someAction(SomeCommandObject sco) {
        // your code...
    }

    // as of GRAILS-11453 if this is used in several 
    // controllers it could be defined in a trait...
    def handleDataBindingException(DataBindingSourceCreationException e) {
        // do whatever makes sense for your application here...
        response.status = 400
        flash.message = 'caught a DataBindingSourceCreationException’
        render view: ‘/someViewWeUseForThisSortOfThing'
    }
}

I think that it makes sense to offer applications the ability to deal with exceptions that are generated while processing command objects if they want to do so.  Some applications might want that sort of thing and for those who don’t, this isn’t imposing any complexity on them.  The code in the framework to support that is minimal.

What say ye?



JSB


—
Jeff Scott Brown
jbr...@gopivotal.com

Find The Cause ~ Find The Cure
http://www.autismspeaks.org/

[zyro]

looks fine for me. allowing the users to catch a
(DataBindingSourceCreation-) Exception if they want to makes much sense.

i would also welcome the widening of this behavior to any Exception
(during command object initialization) delegating the exception handler
lookup to the same code that does it for exceptions thrown from user
code inside controller actions.

thanks again! zyro

[Mathias Fonseca]

I also find this very useful and interesting.

The only observation I would like to make, is that the error added to the object if the DataBindingSourceCreationException is not catched, should be clear and easy to tell apart from other validation errors.

Thanks!