Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global onError doesn't called for FakeApplication (tests) #2484

Closed
maqdev opened this issue Mar 13, 2014 · 10 comments
Closed

Global onError doesn't called for FakeApplication (tests) #2484

maqdev opened this issue Mar 13, 2014 · 10 comments

Comments

@maqdev
Copy link

maqdev commented Mar 13, 2014

Reproduced on Play 2.2.2

  1. Catch all errors and handle them in Global.scala

    import play.api._
    import play.api.mvc.{Results, WithFilters}
    import scala.concurrent.Future
    
    object Global extends GlobalSettings {
        override def onError(request : play.api.mvc.RequestHeader, ex : Throwable) : scala.concurrent.Future[play.api.mvc.SimpleResult] = {
                Future.successful(Results.BadRequest)
        }
    }
    
  2. Generate exception in action

    package controllers
    
    import play.api._
    import play.api.mvc._
    
    object Application extends Controller {
      def index = Action {
        throw new RuntimeException("Unexpected")
        Ok
      }
    }
    
  3. The problem is that if you call that action in browser, you'll get 400 BAD_REQUEST exactly like it's handled in Global.scala, but you can't cover it with test because this test fails with exception:

    @RunWith(classOf[JUnitRunner])
    class ApplicationSpec extends Specification {
    "Application" should {
    "render the index page" in new WithApplication{
    val home = route(FakeRequest(GET, "/")).get
    status(home) must equalTo(BAD_REQUEST)
    }
    }
    }

@jroper
Copy link
Member

jroper commented Mar 14, 2014

Actually, that's a feature - let's say you changed the implementation of Global.onError to return a different error code, then you would have to update every test that you wrote to match that. Your tests for your controllers shouldn't be testing behaviour in Global.onError, if you want to test Global.onError, then write a test that invokes it directly.

Your controller tests should test that the right exception is thrown. This is how most people test, and if the route method did invoke Global.onError, it would prevent them from doing this, they'd have to extract information from the rendered page to try and test that the right exception was thrown, instead of having the exception object to interrogate directly.

It's the same reason why we don't invoke Global.onHandlerNotFound if no route is found, instead we return Option, to give more power to your tests.

But if you don't want this flexibility, then that's easy to address, simply write your own route helper that looks like this:

def routeWithOnError[A](req: Request[A]): Option[Future[SimpleResult]] = {
  route(req).map { result =>
    result.recoverWith {
      case t: Throwable => Global.onError(req, t)
    }
  }
}

@nik-kashi
Copy link
Contributor

Thanks @jroper
What about java?
How can I set onError in Global as exception handler in java testing?

@zarinfam
Copy link

You can use something like the following code in java 8:

public Result routeWithOnError(FakeRequest fakeRequest){
    F.Promise<Result> promise = F.Promise.promise(() -> route(fakeRequest));
    F.Function<Throwable, F.Promise<Result>> f = throwable -> global.onError(null, throwable);
    return promise.recoverWith(f).get(100);
}

@nik-kashi
Copy link
Contributor

Thanks @zarinfam

@mfarhans
Copy link

The post is very helpful. I am writing tests (using FakeApplication & FakeRequest) for controllers (for which the global exceptions are handled in the Global.onError() method), in which I want include/cover the execution of {{Global.onError method}}.

I am using play framework in java, and missing something rather basic here, and that is how do you obtain the "global" reference in the java example of routeWithOnError given above. @mohsenkashi (or someone) could share a fully functional route(), if the approached worked?

@zarinfam
Copy link

For test you can write a new Global class and send an instance of it to fakeApplication() method:

    Global global =new Global4Test();
    FakeApplication fakeApplication = fakeApplication(inMemoryDatabase(),global);
    start(fakeApplication);

or you can get real global of your project using this code:

   fakeApplication.getWrappedApplication().global();

@zarinfam
Copy link

Java 8 version of routeWithOnError for play 2.4+:

public static Result routeWithOnError(Http.RequestBuilder requestBuilder) throws InterruptedException, ExecutionException, TimeoutException {
    CompletableFuture<Result> future = CompletableFuture.supplyAsync(() -> route(requestBuilder));

    BiFunction<Result, Throwable, CompletionStage<Result>> f =
            (r, t) -> (t == null)
                    ? CompletableFuture.supplyAsync(() -> r)
                    : new CustomHttpErrorHandler().onError(null, t);

    return future.handleAsync(f).thenCompose(x -> x).get(20000, TimeUnit.MILLISECONDS);
}

@EdgeCaseBerg
Copy link
Contributor

For play scala 2.4 with an ErrorHandler bound to the application via a GuiceApplicationBuilder, you still need to call it with a helper method.

import play.api.http.Writeable
import play.api.mvc.{ Result, Request }
def routeWithOnError[A: Writeable](req: Request[A]): Option[Future[Result]] = {
		route(req).map { result =>
			result.recoverWith {
				case t: Throwable => errorHandler.onServerError(req, t)
			}
		}
	}

@scadgek
Copy link
Contributor

scadgek commented Sep 20, 2017

Hi @jroper,

Just wanted to inquire whether you haven't changed your position after 3 years. I can find a few counterarguments to your claim that we don't need to test onError()'s behaviour when testing controllers. In my view, when I test a controller, what I want to accomplish is to emulate someone issuing an HTTP request to my service and test the result he receives, which is in this case the result on onError()'s execution. At the same time, if I want to test that the proper exception is thrown, I'll test the underlying controller's class, and this will also encourage decoupling of business logic from the router's logic.

Of course, the approach you suggested - writing tests for the HttpErrorHandler itself - would also work, but I believe this makes testing the controller either redundant or incomplete, since it does nothing besides parsing request and by itself does not throw any exceptions - instead it returns Result instances.

Please tell us what you think of this, and thanks in advance!

@sb-yb
Copy link

sb-yb commented Mar 15, 2021

It would be nice to document this lack of custome error handler as part of
https://www.playframework.com/documentation/2.8.x/JavaErrorHandling
and
https://www.playframework.com/documentation/2.8.x/ScalaErrorHandling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants