quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

Make CheckedTemplate support exceptions

Open gbourant opened this issue 1 year ago • 2 comments

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 avatar Feb 18 '24 09:02 gbourant

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

gbourant avatar Feb 18 '24 09:02 gbourant

/cc @mkouba (qute)

quarkus-bot[bot] avatar Feb 19 '24 07:02 quarkus-bot[bot]

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?

FroMage avatar Feb 20 '24 09:02 FroMage

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.

gbourant avatar Feb 20 '24 10:02 gbourant

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).

FroMage avatar Feb 20 '24 10:02 FroMage

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 :)

gbourant avatar Feb 20 '24 11:02 gbourant

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 :)

FroMage avatar Feb 20 '24 14:02 FroMage

Alright, I opened an issue in Renarde :)

gbourant avatar Feb 21 '24 04:02 gbourant

Thanks

FroMage avatar Feb 21 '24 14:02 FroMage