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

Make CheckedTemplate support exceptions #38839

Closed
gbourant opened this issue Feb 18, 2024 · 9 comments
Closed

Make CheckedTemplate support exceptions #38839

gbourant opened this issue Feb 18, 2024 · 9 comments
Labels
area/qute The template engine kind/enhancement New feature or request

Comments

@gbourant
Copy link

gbourant commented Feb 18, 2024

Description

When i return TemplateInstance in my Renarde application it means that any exception that i catch in a try catch block is not rollbacked.

In the following example the transaction is not rollbacked.

@CheckedTemplate
public class AdminTemplate {
    public static native TemplateInstance auth(Exception e);
    public static native TemplateInstance auth();
}
@Path("login")
public TemplateInstance loginPage(User user)
        try {
            authService.register(user);
        } catch (Exception e) {
            // transaction is not rollbacked
            return AdminTemplate.auth(e);
        }
        return AdminTemplate.auth();
}

In order to fix this issue we could change the AdminTemplate to support "exceptions". One way could be to add a new return type ExceptionTemplateInstance. Another way could be to have a special name auth$Exception() (or something like that), or a method with throws authException() throws WebApplicationException or add a new @Rollback annotation to the method. In all cases instead of just rendering the template, it would throw WebApplicationException and as it's content it would be the rendered template.

@CheckedTemplate
public class AdminTemplate {
    public static native TemplateInstance auth(Exception e);
    public static native TemplateInstance auth();
    public static native TemplateInstance authException(Exception e) throws WebApplicationException;
    public static native TemplateInstance auth$Exception(Exception e);
    public static native ExceptionTemplateInstance exceptionAuth(Exception e);
    `@Rollback` `@Location("auth")`
    public static native TemplateInstance authError(Exception e);
}
@Path("login")
public ExceptionTemplateInstance loginPage(User user)
        try {
            authService.register(user);
        } catch (Exception e) {
            // transaction is rollbacked
            return AdminTemplate.auth$Exception(e); // or return AdminTemplate.exceptionAuth(e); // or return AdminTemplate.authException(e); // or return AdminTemplate.authError(e);
        }
       return AdminTemplate.auth();
}

This is more or less my brainstorming ideas, what do you think?

Implementation ideas

No response

@gbourant gbourant added the kind/enhancement New feature or request label Feb 18, 2024
@gbourant
Copy link
Author

//cc: Maybe this is relevant @FroMage, if not sorry for the tag.

@yrodiere yrodiere added area/qute The template engine and removed triage/needs-triage labels Feb 19, 2024
Copy link

quarkus-bot bot commented Feb 19, 2024

/cc @mkouba (qute)

@FroMage
Copy link
Member

FroMage commented Feb 20, 2024

I'm not sure what you mean here. Exceptions thrown during the controller's endpoint method will cause a rollback of the transaction, whether they return a TemplateInstance or not.

It's possible that exceptions that occur during rendering of the template, which happens after the endpoint method is called, do not cause a rollback, because the transaction has been committed at this time. Perhaps this is what you're referring to?

@gbourant
Copy link
Author

I'm not sure what you mean here. Exceptions thrown during the controller's endpoint method will cause a rollback of the transaction, whether they return a TemplateInstance or not.

I agree, generally the rollback works as excepted when an exception is thrown, but in this case i use a try catch block in order to display the correct error message based on the exception so it won't cause the transaction to rollback.

In order for the transaction to rollback i would have to rethrow an exception like this:

@Path("login")
public TemplateInstance loginPage(User user)
        try {
            authService.register(user);
        } catch (Exception e) {
            // transaction is rollbacked
            var response = Response
                        .ok()
                        .entity(AdminTemplate.auth(e))
                        .build();
            throw new WebApplicationException(response);
        }
       return AdminTemplate.auth();
}

So, instead of rethrowing the exception, a special CheckedTemplate (type,annotation,etc,) could inform the transaction to rollback.

@FroMage
Copy link
Member

FroMage commented Feb 20, 2024

Oh, I see what you mean, you want to handle the exception but force a rollback.

How about https://quarkus.io/guides/transaction#declarative-approach:

    @Inject TransactionManager tm; 

@Path("login")
public TemplateInstance loginPage(User user)
        try {
            authService.register(user);
        } catch (Exception e) {
           tm.setRollbackOnly();
            return AdminTemplate.auth(e);
        }
        return AdminTemplate.auth();
}

This should work if you're using Hibernate ORM (not reactive).

@gbourant
Copy link
Author

That is definitely working (just tested it in Renarde).

What i'm looking for is more about the Developer Joy (and less error prone, forget calling the rb method) otherwise i could use either of the following two static methods:

public static TemplateInstance rb(TemplateInstance templateInstance){
        try {
            Arc.container().instance(TransactionManager.class).get().setRollbackOnly();
        } catch (SystemException e) {
            throw new RuntimeException(e);
        }
        return templateInstance;
}

public static TemplateInstance rb(TemplateInstance templateInstance){
        var response = Response
                .ok()
                .entity(templateInstance)
                .build();
        throw new WebApplicationException(response);
}
@Path("login")
public TemplateInstance loginPage(User user)
        try {
            authService.register(user);
        } catch (Exception e) {
            return rb(AdminTemplate.auth(e));
        }
        return AdminTemplate.auth();
}

Well, if this is out of the scope i think i can live with that :)

@FroMage
Copy link
Member

FroMage commented Feb 20, 2024

I suppose I could add a Transaction.rollback() static method in Renarde, if you prefer it over injection. It could also be an instance method of Controller, perhaps. In any case, all these options seem less complicated than the original proposal, do you agree? If yes, then feel free to open an issue in Renarde :)

@gbourant
Copy link
Author

Alright, I opened an issue in Renarde :)

@FroMage
Copy link
Member

FroMage commented Feb 21, 2024

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/qute The template engine kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants