ninjaframework / ninja

Ninja is a full stack web framework for Java. Rock solid, fast and super productive.
http://www.ninjaframework.org
Apache License 2.0
1.91k stars 520 forks source link

Removing the TemplateEngineFreemarker binding causes non-existing routes to return 200 OK #486

Open omervk opened 8 years ago

omervk commented 8 years ago

I've started using NinjaClassicModule to remove parts of Ninja Framework I didn't need. One of those is the templating engine Freemarker, since my server only ever returns JSON responses.

Assume my server exposes a route /foo/bar and only accepts POST requests. Calling it with the following returns 200 when it should return 404:

GET /foo/bar
Accept: text/html

Here's the stack trace from my logs:

13:37:01.518 [http-nio-8080-exec-6] ERROR ninja.NinjaDefault - Unable to handle result. That's really really fishy.
ninja.exceptions.NinjaException: No template engine found for result content type text/html
    at ninja.utils.ResultHandler.renderWithTemplateEngineOrRaw(ResultHandler.java:122)
    at ninja.utils.ResultHandler.handleResult(ResultHandler.java:78)
    at ninja.NinjaDefault.renderErrorResultAndCatchAndLogExceptions(NinjaDefault.java:136)
    at ninja.NinjaDefault.onRouteRequest(NinjaDefault.java:122)
    at ninja.servlet.NinjaServletDispatcher.service(NinjaServletDispatcher.java:86)
    at com.google.inject.servlet.ServletDefinition.doServiceImpl(ServletDefinition.java:287)
    at com.google.inject.servlet.ServletDefinition.doService(ServletDefinition.java:277)
    at com.google.inject.servlet.ServletDefinition.service(ServletDefinition.java:182)
    at com.google.inject.servlet.ManagedServletPipeline.service(ManagedServletPipeline.java:91)
    at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:85)
    at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:119)
    at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:133)
    at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:130)
    at com.google.inject.servlet.GuiceFilter$Context.call(GuiceFilter.java:203)
    at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:130)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:240)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:207)
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:212)
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:106)
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:141)
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
    at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:616)
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88)
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:522)
    at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1095)
    at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:672)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1500)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1456)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
    at java.lang.Thread.run(Thread.java:745)
jjlauer commented 8 years ago

Hi Omer,

The FrameworkModule is a new feature and I'm the one that implemented it, so I'm glad to help. My main use case for the FrameworkModule feature was replacing Freemarker with a different html template engine -- and being able to remove the Freemarker dependency. Before 5.4.0, you could plug a new template engine in, but you'd still need to have the freemarker jar on your classpath. I think you're use-case is also good -- where you don't even want text/html support.

The reason you're seeing that error is that by default Ninja's default error handling still uses freemarker templates. Template engines that support text/html happen to override those default templates. When you have no text/html template engine, Ninja doesn't know what to do by default.

Is this ideal? No. I think the idea of using Ninja as a json-only backend is a good idea. I'd guess in a future version it'd make sense to improve the error handling code to maybe have a fallback, simple text/html template engine or if json is the only template engine, perhaps just send back a json error page. I'm sure everyone in the Ninja community would appreciate a pull request with the enhancement. I probably will hit this same use case soon, so I'll probably implement improvements around this in the new few months as well.

I can think of a couple workarounds for 5.4.0.

1) You can implement your own custom Ninja class to customize how any default error result is returned. http://www.ninjaframework.org/documentation/custom_routing.html.

2) Enable a text/html template engine. If you want light weight, then I'd suggest Rocker templates. https://github.com/fizzed/ninja-rocker

-Joe

On Tue, Mar 15, 2016 at 7:46 AM, Omer van Kloeten notifications@github.com wrote:

I've started using NinjaClassicModule to remove parts of Ninja Framework I didn't need. One of those is the templating engine Freemarker, since my server only ever returns JSON responses.

Assume my server exposes a route /foo/bar and only accepts POST requests. Calling it with the following returns 200 when it should return 404:

GET /foo/barAccept: text/html

Here's the stack trace from my logs:

13:37:01.518 [http-nio-8080-exec-6] ERROR ninja.NinjaDefault - Unable to handle result. That's really really fishy. ninja.exceptions.NinjaException: No template engine found for result content type text/html at ninja.utils.ResultHandler.renderWithTemplateEngineOrRaw(ResultHandler.java:122) at ninja.utils.ResultHandler.handleResult(ResultHandler.java:78) at ninja.NinjaDefault.renderErrorResultAndCatchAndLogExceptions(NinjaDefault.java:136) at ninja.NinjaDefault.onRouteRequest(NinjaDefault.java:122) at ninja.servlet.NinjaServletDispatcher.service(NinjaServletDispatcher.java:86) at com.google.inject.servlet.ServletDefinition.doServiceImpl(ServletDefinition.java:287) at com.google.inject.servlet.ServletDefinition.doService(ServletDefinition.java:277) at com.google.inject.servlet.ServletDefinition.service(ServletDefinition.java:182) at com.google.inject.servlet.ManagedServletPipeline.service(ManagedServletPipeline.java:91) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:85) at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:119) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:133) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:130) at com.google.inject.servlet.GuiceFilter$Context.call(GuiceFilter.java:203) at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:130) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:240) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:207) at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:212) at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:106) at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:141) at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79) at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:616) at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88) at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:522) at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1095) at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:672) at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1500) at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1456) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) at java.lang.Thread.run(Thread.java:745)

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/ninjaframework/ninja/issues/486

omervk commented 8 years ago

Thanks for the detailed reply, @jjlauer. For now I re-enabled Freemarker as a workaround, since removing it is not critical for me to remove it at this time.

For now, let's keep this issue open until it's fixed.