MyFaces Core
  1. MyFaces Core
  2. MYFACES-3309

Throw correct exception while using FactoryFinderProvider SPI

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.9, 2.1.3
    • Fix Version/s: 2.0.10, 2.1.4
    • Component/s: SPI
    • Labels:
      None

      Description

      While using FactoryFinderProvider SPI, it is required to check the real exception from InvocationTargetException, and throw some exceptions directly.

        Activity

        Hide
        Leonardo Uribe added a comment -

        I checked the patch and the changes proposed are reasonable. Thanks to Ivan for provide this patch.

        Show
        Leonardo Uribe added a comment - I checked the patch and the changes proposed are reasonable. Thanks to Ivan for provide this patch.
        Hide
        Ivan added a comment - - edited

        Sometimes, those exceptions are thrown from runtime. e.g. If users called FactoryFinder.getFactory("invalidfactory"), there should be an IllegalStateException exception is thrown. If it is not possible to have a new release, could you please help to review the patch ? I wonder whether I missed anything. So that, I could try to do something from Geronimo side.
        Thanks.

        Show
        Ivan added a comment - - edited Sometimes, those exceptions are thrown from runtime. e.g. If users called FactoryFinder.getFactory("invalidfactory"), there should be an IllegalStateException exception is thrown. If it is not possible to have a new release, could you please help to review the patch ? I wonder whether I missed anything. So that, I could try to do something from Geronimo side. Thanks.
        Hide
        Leonardo Uribe added a comment -

        In theory if an exception is thrown in that location, that means initialization should be aborted and the web application should not be started. Maybe the problem could be seen in a different way. There is no mechanism in MyFaces to check if a setup for a web application is correct. So if something wrong, it just fails without an explanation. Again, it looks like an improvement. Since the base case using the default algorithm works as expected, I think this issue can wait until next version scheduled in 2 months (or unless another issue with high priority comes out).

        Show
        Leonardo Uribe added a comment - In theory if an exception is thrown in that location, that means initialization should be aborted and the web application should not be started. Maybe the problem could be seen in a different way. There is no mechanism in MyFaces to check if a setup for a web application is correct. So if something wrong, it just fails without an explanation. Again, it looks like an improvement. Since the base case using the default algorithm works as expected, I think this issue can wait until next version scheduled in 2 months (or unless another issue with high priority comes out).
        Hide
        Ivan added a comment -

        Not sure I miss anything, it will cause some JSF compatible cases failed, and did not see there is a way to solve in in integration codes.

        Show
        Ivan added a comment - Not sure I miss anything, it will cause some JSF compatible cases failed, and did not see there is a way to solve in in integration codes.
        Hide
        Leonardo Uribe added a comment -

        I don't think so. This looks like an improvement, instead a bug. It is true the try/catch block wraps all exceptions into a FacesException,and instead it should throw the exceptions like the javadoc of FactoryFinder says. Anyway, the real cause can be found using getCause(), so in practice it is possible to add some code that check that.

        Show
        Leonardo Uribe added a comment - I don't think so. This looks like an improvement, instead a bug. It is true the try/catch block wraps all exceptions into a FacesException,and instead it should throw the exceptions like the javadoc of FactoryFinder says. Anyway, the real cause can be found using getCause(), so in practice it is possible to add some code that check that.
        Hide
        Ivan added a comment -

        If it is really a bug, is it possible for the community to publish a new 2.0.x ? Or is there a workaround that I could do for it ? Thanks.

        Show
        Ivan added a comment - If it is really a bug, is it possible for the community to publish a new 2.0.x ? Or is there a workaround that I could do for it ? Thanks.
        Hide
        Ivan added a comment -

        Provide a proposed patch file.

        Show
        Ivan added a comment - Provide a proposed patch file.

          People

          • Assignee:
            Leonardo Uribe
            Reporter:
            Ivan
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development