• Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Highest Highest
    • 4.3
    • 4.3
    • None
    • Tested on Safari 5.1.7 and Firefox 13.0.1

      Typing the '{' character in the editor only shows "Open Macro Browser" option, upon opening the macro browser an error is shown There has been an error loading the macro browser. Please try again or see your system administrator.

      Nothing is shown in the logs during the time, and nothing is sent over the network.

      Once I click "Discard" the exceptions in the attached log are emitted with the following line:

      2012-07-13 06:28:43,839 WARN [TP-Processor5] [confluence.pages.actions.AbstractCreateAndEditPageAction] getCancelResult Removing invalid draft: Draft = { id: 4653218, type: page, title: Indentation Tests, pageId: 4653217}, for user : EmbeddedCrowdUser{name='BRYAN', displayName='Brett Ryan', directoryId=22872065} 
      

            [CONFSERVER-26014] Macro browser does not work in 4.3-beta1

            I had a chance to check this against 4.3-rc3, and the older version of our plugin no longer causes Confluence to blow up. Thanks, Craig and Brett!

            Scott Dudley [Inactive] added a comment - I had a chance to check this against 4.3-rc3, and the older version of our plugin no longer causes Confluence to blow up. Thanks, Craig and Brett!

            Craig,

            Great! Thanks for catching that other (new) class too. We'll make sure to test against 4.3-rc1 when it comes out.

            Thanks again.

            Scott

            Scott Dudley [Inactive] added a comment - Craig, Great! Thanks for catching that other (new) class too. We'll make sure to test against 4.3-rc1 when it comes out. Thanks again. Scott

            sdudley

            I've removed the final restrictions on the RemoveAttachedFileAction and RemoveAttachedFileVersionAction classes.

            In my testing this resolves the start up issues and problems with the macro browser.

            I'd suggest you do some compatibility testing for this plugin once 4.3-rc1 is available (either late this week or early next week).

            Petch (Inactive) added a comment - sdudley I've removed the final restrictions on the RemoveAttachedFileAction and RemoveAttachedFileVersionAction classes. In my testing this resolves the start up issues and problems with the macro browser. I'd suggest you do some compatibility testing for this plugin once 4.3-rc1 is available (either late this week or early next week).

            sdudley I'm expecting to take it out, but I just need to confirm there wasn't a good reason why it was added. The engineer who made the change was caught up with some production work on Friday, so I should have an answer on Monday (Sydney time).

            Petch (Inactive) added a comment - sdudley I'm expecting to take it out, but I just need to confirm there wasn't a good reason why it was added. The engineer who made the change was caught up with some production work on Friday, so I should have an answer on Monday (Sydney time).

            Craig,

            Great. Once you have that discussion, could you please let me know, either way, what is going to happen with the "final" keyword? If we're going to have to engineer around it, we'd love to schedule that work sooner rather than later.

            If "final" stays, I'm a little worried that some people may not upgrade their plugins until after they upgrade the baseline product. Given the severity of the classloader/XWork bug and how badly it breaks the rest of the system, this could trigger a bunch of support calls to you and/or us that we might otherwise be able to avoid.

            If you do want "final" there for idealistic reasons (but without current functional need), perhaps we can work with you to phase it back in after a couple of releases. I'd ideally like to get the majority of our users weaned off the incompatible plugin version before you make that change...or, even better, introduce it at the same time as your exception-catching code that prevents Xwork from falling apart.

            Thanks!
            Scott

            Scott Dudley [Inactive] added a comment - Craig, Great. Once you have that discussion, could you please let me know, either way, what is going to happen with the "final" keyword? If we're going to have to engineer around it, we'd love to schedule that work sooner rather than later. If "final" stays, I'm a little worried that some people may not upgrade their plugins until after they upgrade the baseline product. Given the severity of the classloader/XWork bug and how badly it breaks the rest of the system, this could trigger a bunch of support calls to you and/or us that we might otherwise be able to avoid. If you do want "final" there for idealistic reasons (but without current functional need), perhaps we can work with you to phase it back in after a couple of releases. I'd ideally like to get the majority of our users weaned off the incompatible plugin version before you make that change...or, even better, introduce it at the same time as your exception-catching code that prevents Xwork from falling apart. Thanks! Scott

            sdudley thanks for the quick reply and finding the class you were extending that was causing the problem.

            I agree that such a failure shouldn't bring down so much of the system, we'll see if there's a way we can improve that, though not for 4.3.0 since we are getting close to freezing the source.

            There was definitely a change of this class to final, and I'm trying to find out why from the committer.

            Mostly likely we'll just take it off again for 4.3 unless there's a good reason.

            Petch (Inactive) added a comment - sdudley thanks for the quick reply and finding the class you were extending that was causing the problem. I agree that such a failure shouldn't bring down so much of the system, we'll see if there's a way we can improve that, though not for 4.3.0 since we are getting close to freezing the source. There was definitely a change of this class to final, and I'm trying to find out why from the committer. Mostly likely we'll just take it off again for 4.3 unless there's a good reason.

            Brett Ryan added a comment -

            Thanks both Scott and Craig for looking into this one.

            Another symptom is that most of the admin screens in confluence can not be accessed stating that the page does not exist. At first I thought this was unrelated and started investigating the cause, though upon getting the macro browser to start working again I also realised that the rest of the system became stable.

            Your explanation Scott is a very good one and I would agree that any failures within plugins should not bring half the system down.

            Thanks again, and we look forward to the final 4.3 release

            Brett Ryan added a comment - Thanks both Scott and Craig for looking into this one. Another symptom is that most of the admin screens in confluence can not be accessed stating that the page does not exist. At first I thought this was unrelated and started investigating the cause, though upon getting the macro browser to start working again I also realised that the rest of the system became stable. Your explanation Scott is a very good one and I would agree that any failures within plugins should not bring half the system down. Thanks again, and we look forward to the final 4.3 release

            At first, I thought that this issue was not caused by us: I installed Lockpoint on a virgin 4.3-beta-1 install, and while the above error message did appear in the logs upon our load (which is something that we'd investigate regardless), the macro browser was still working fine.

            However, after a Confluence restart, I was able to see the behavior described here. Lockpoint adds some methods to Xwork, and it looks like the Confluence Xwork handler gets confused if a runtime exception gets thrown in the middle of adding a new Xwork action. The order of execution is apparently significant, since this was an issue for me when Lockpoint was installed on startup but not if it was loaded at runtime.

            As for the root cause, this seems to be related to a Confluence API change (of sorts). The error message above is rather unhelpful, but after taking some educated guesses, I think the problem is that the "final" keyword crept into Confluence 4.3's definition of RemoveAttachedFileAction:

            public final class RemoveAttachedFileAction extends AbstractRemoveAttachmentAction
            

            Lockpoint adds an Xwork action (/lockpoint/removeattachment.action) using a handler that extends the above class, so the classloader's unhappiness while manipulating the action table seemingly blows other parts of Confluence out of the water. Note that Lockpoint isn't touching anything that is even remotely near the macro browser--it looks like our failed attempt to add an unrelated Xwork action is causing all of the problems.

            To me, this suggests two things:

            1- Is the final keyword there for a specific reason? If not, doing "final" would save us a bunch of work.

            2- It seems like Confluence might do a better job of catching this exception in the first place (in XWorkModuleDescriptor.addAction?), such that even if a plugin explodes due to a runtime issue, it should still leave the Xwork action table in a consistent state.

            Scott Dudley [Inactive] added a comment - At first, I thought that this issue was not caused by us: I installed Lockpoint on a virgin 4.3-beta-1 install, and while the above error message did appear in the logs upon our load (which is something that we'd investigate regardless), the macro browser was still working fine. However, after a Confluence restart, I was able to see the behavior described here. Lockpoint adds some methods to Xwork, and it looks like the Confluence Xwork handler gets confused if a runtime exception gets thrown in the middle of adding a new Xwork action. The order of execution is apparently significant, since this was an issue for me when Lockpoint was installed on startup but not if it was loaded at runtime. As for the root cause, this seems to be related to a Confluence API change (of sorts). The error message above is rather unhelpful, but after taking some educated guesses, I think the problem is that the "final" keyword crept into Confluence 4.3's definition of RemoveAttachedFileAction: public final class RemoveAttachedFileAction extends AbstractRemoveAttachmentAction Lockpoint adds an Xwork action (/lockpoint/removeattachment.action) using a handler that extends the above class, so the classloader's unhappiness while manipulating the action table seemingly blows other parts of Confluence out of the water. Note that Lockpoint isn't touching anything that is even remotely near the macro browser--it looks like our failed attempt to add an unrelated Xwork action is causing all of the problems. To me, this suggests two things: 1- Is the final keyword there for a specific reason? If not, doing " final " would save us a bunch of work. 2- It seems like Confluence might do a better job of catching this exception in the first place (in XWorkModuleDescriptor.addAction?), such that even if a plugin explodes due to a runtime issue, it should still leave the Xwork action table in a consistent state.

            sdudley are you aware of any compatibility issues with Arsenalte Lockpoint and Confluence 4.3 beta 1?

            Cheers,
            Craig

            Petch (Inactive) added a comment - sdudley are you aware of any compatibility issues with Arsenalte Lockpoint and Confluence 4.3 beta 1? Cheers, Craig

            Brett Ryan added a comment -

            My bad, it's actually Arsenale Lockpoint - 1.3.0

            Brett Ryan added a comment - My bad, it's actually Arsenale Lockpoint - 1.3.0

              Unassigned Unassigned
              6c4fdac73624 Brett Ryan
              Affected customers:
              0 This affects my team
              Watchers:
              3 Start watching this issue

                Created:
                Updated:
                Resolved: