casid / jte

Secure and speedy templates for Java and Kotlin.
https://jte.gg
Apache License 2.0
825 stars 62 forks source link

Throwing checked exceptions in JTE #91

Closed izogfif closed 3 years ago

izogfif commented 3 years ago

My JSP code calls many methods that throw checked exception. It's actually OK because if an exception is thrown during HTML code generation, it is caught and redirect is sent instead of HTTP 200 with HTML code. For example, resource requested by user is no longer valid / no longer exists. Therefore instead of showing requested page, user is redirected to special page (404 page or item list page - doesn't matter). Upon converting to JTE I get compilation errors because Java code created from .jte-files doesn't allow throwing any exception. And I can't write

!{try{}
${controller.getResourceUrl(id);}
!{catch(ResourceNotFoundException e) { throw new WrappedUncheckedException(e);}

Would it be possible to add throws Exception or throws Throwable to Java code generated by JTE?

casid commented 3 years ago

Hey @izogfif, uugh that might be possible, but I'm not a fan of it.

First, this would be a breaking change, as Content signature would have to change to be able to throw Exceptions, while at the moment, all Content implementations in the wild don't do that.

Second, it makes exception handling in custom Content implementations blocks less explicit, since every checked exception whatsoever will just be passed through.

An alternative might be to create your own RuntimeException implementations (like some ResourceNotFound) and throw that instead of the checked exception. Or, if you don't really use the exceptions for recovering, there's also UncheckedIoException.

izogfif commented 3 years ago

@casid OK. Maybe add @throws keyword that can only be specified once at the start of .jte file? Similar to @import and @param. What's the standard way to use / call code / methods that throw checked exceptions in .jte files?

casid commented 3 years ago

I'm still not convinced that this is good design. Wouldn't it be better to not have checked exceptions in code called by UI at all?

I never had the need for checked exceptions in any project I used jte before (in the API that jte templates use).

I don't understand your example. Why does ${controller.getResourceUrl} has to throw a checked exception? What happens if you render a list of resources and one does not exist?

izogfif commented 3 years ago

Why does ${controller.getResourceUrl} has to throw a checked exception?

getResourceUrl is used not only in .jte file, but also in Java files. Checked exceptions are needed in Java code for various reasons. Writing wrapper / copies of methods that throw unchecked exceptions / don't throw exceptions at all and can only be used in .jte is cumbersome (there are dozens of such methods).

What happens if you render a list of resources and one does not exist?

The exception is thrown, caught, and the user is redirected to "Not found" page (in case if the resource was removed from the system, and an error is logged). This might happen if the list of resources was taken from the cache and these particular records were invalidated in the cache before the rendering of HTML page finished.

casid commented 3 years ago

It might be cumbersome, but doesn't it lead to a much better user experience? Since you have to explicitly decide what should happen in case of such an exception? And if you don't want to react, shouldn't it be an unchecked exception instead? The way you describe how those exceptions work, sounds a lot like unchecked exceptions.

I don't know how your system works, but in case of a missing resource within a list, wouldn't it be better to render the list without the missing resource, instead of showing the user an error for the entire page?

Checked Exceptions in frontend near code are not very popular in modern Java. I'm not a fan of encouraging their use through jte.