[play-2.0] best way to handle errors in controllers with scala

[sveri]

Hi,

i cannot find docs about the concrete error handling in the controllers. I did find: http://www.playframework.org/documentation/2.0/ScalaTodoList but it's a little bit vague i guess.

Let's say i have this controller:

def newTasks = Action { implicit request =>
    try{
        val tasks = taskForm.bindFromRequest.get
        for(task <- tasks.tasks){
            Context.executeInNew {ctx =>
           
            println("done: " +  task.done())
            task.save()
            }
        }
    }catch{
     // doesnt work cause of "method newTasks has return statement; needs result type"
     case e: Exception => return BadRequest
     
    }
    Ok("")
  }

Then i cannot return BadRequest in case of error. I need to specify a return type.
I tried to use "Status, Result and AnyRef" as return types, but none of them work, due to different errors.

What is the right way to return a different Status if something goes wrong? I dont want to do a redirect, as i am calling this method from a ajax method with jquery and need to act accordingly, depending on the return type.

Thanks in Advance,
Sven

[Julien Richard-Foy]

You can just add a return type to your newTasks action…

But a better way would be to return the Ok(…) from the try branch
instead of outside your try/catch expression.

[sveri]

Hi Julien,

but which return type? Like i wrote, "Status, Result and AnyRef" dont work, as they throw different errors.

Best Regards,
Sven

[Julien Richard-Foy]

An action is a computation producing a result from a request, so
writing Result should work.
anyway, you don’t need to explicitly write the return type of your
action, and it’s actually more Scala idiomatic to not write the
“return” keyword. Just move the `Ok` at the end of the try bloc (just
before the catch block), and remove the return keyword.

[virtualeyes]

agreed, but when you have various embedded if/else branches within a
single Action method, then not being able to use "return" to exit
processing becomes a hassle.

def save = Action { implicit request =>
val bound = form.bindFromRequest
if(bound.hasErrors) return Ok(withForm(bound))

// no need for else {} branch, so don't have to indent into deeper
and deeper levels.
val model = bound.get
if(!processPayment(model)) return
Ok( withForm(bound.withGlobalError(...) ))

// again no need for else nesting, return escapes further processing
}

Yes, could abstract out into helper methods, but then you have to
specify param signatures and add boilerplate for code that logically
belongs within Action method block.

btw, specifying return type of Action method then requires specifying
type of implicit request = hassle

Don't get me wrong, Play completely freaking rocks, just not in this
particular case ;-) (or I am missing a workaround...)

[johanandren]

If you think about the problem in a more functional way (instead of the procedural/imperative - "escape with a return", which in my opinion isn't a very good solution for any codebase). Each expression always has a return value, which in your case is either Ok or BadRequest, i'm using fold as well as that will handle the extraction of model and gives me a way to write names for the data that each possible outcome gives me (hasErrors is the form-object with errors, model is the output of form.get on a valid form):

def save = Action { implicit request => 

  form.bindFromRequest.fold(

    formWithErrors => BadRequest(withForm(formWithErrors)),

    model => {

      if(processPayment(model)) { 

        Ok(... success ...)

      } else {

        BadRequest(withForm(bound.withGlobalError(...) )) 
      }

   })

} 

Now you can think about each block/expression/nesting-level as a function that returns something, and that something will be returned for each block all the way up to your save action.

This, to me at least, becomes way more readable than a function with three returns at different points. Hope this was any help!

[virtualeyes]

Johan,

yes, trying to extract myself out of the imperative approach,
something I've managed to do with persistence:
db.handle withSession { implicit ss: Session=>
ss.withTransaction {
val result = for {
u <- safeInsert( User.insert( user ) )
m <- safeInsert( Membership.insert( member ) )
o <- safeInsert( Order.insert( order) )
} yield (u, m, o)
result match {
case Some( (u, m, o) ) =>
Right( (userID, orderID, orderNum) )
case _ => ss.rollback; Left("subscription.not.created")
} } }

however, at the controller layer you may have several conditions to
handle which cannot easily be combined into an elegant control flow
structure. For example, when creating a new subscription the user can
by credit card or by check, so with the above persistence block (in
DAO layer) there are at least 3 conditionals to handle.

It's petty, but I'm not a big fan of indenting down the conditional
rabbit hole. I know I can extract business logic into helper methods,
but my point is more that in some cases, imperative + return is make
for more elegant looking code...whether it is better, that's not a
winnable debate in a language like Scala ;-)

Thanks for the tip, I'm refactoring now, going to see how the fold
with embedded anonymous block works, should be doable, it's a tiny
chunk of code in reality....