eclipse-aspectj / aspectj

Other
291 stars 84 forks source link

java.lang.RuntimeException: key not found in wovenClassFile #245

Closed afathonih closed 1 year ago

afathonih commented 1 year ago

My team test project start seeing this issue. I have seen others reported but there's still no solution because there's no reproducibility method.

The following repo can reproduce it in quite consistent manner: https://github.com/afathonih/ajcwovenissue

We need to run it around 50 times. And the problem will crop up roughly twice.

kriegaex commented 1 year ago
[ERROR] Failed to execute goal on project processor: Could not resolve dependencies for project org.example:processor:jar:1.0-SNAPSHOT: com.rakuten.linkconsumer:rhapsody-framework:pom:0.0.24-SNAPSHOT was not found in https://repo1.maven.org/maven2 during a previous attempt. This failure was cached in the local repository and resolution is not reattempted until the update interval of central has elapsed or updates are forced -> [Help 1]

Please either add a snapshot repository definition to the project or let me know which non-snapshot I can use instead.

afathonih commented 1 year ago

Sorry. I thought I have cleaned-up this non-dep. I have removed the above dependency and it still still reproducible.

kriegaex commented 1 year ago

I was able to reproduce the problem with each run, I did not have to run 50 times. The problem was AspectJ Maven misconfiguration. Aspect libraries should not be declared as plugin dependencies but simply as normal Maven module dependencies.

Also, Surefire and Failsafe should not be declared as module dependencies (your application does not need them on the classpath), only as plugins.

See PR https://github.com/afathonih/ajcwovenissue/pull/1.

afathonih commented 1 year ago

Thank you for looking into this and also for the enlightenment. I will apply this solution into the actual project and hopefully no more exception triggered.

kriegaex commented 1 year ago

@afathonih wrote in https://github.com/afathonih/ajcwovenissue/pull/1#issuecomment-1606845237:

@kriegaex I managed to setup GH action and run your PR there: https://github.com/afathonih/ajcwovenissue/actions/runs/5372836175/jobs/9746611803?pr=1

It's still reproducible.

Therefore, I am reopening the issue.

Quick link to the GitHub workflow definition running the build multiple times: https://github.com/afathonih/ajcwovenissue/blob/main/.github/workflows/ci.yml

kriegaex commented 1 year ago

@afathonih, locally I noticed that I can reproduce the problem sometimes with your configuration, just like you said. But then I noticed that I had overlooked a few things:

  1. The Allure aspects are for test reporting only, i.e. they should be woven only during test-compile, not compile. The dependencies should also be test-scoped. Correction: allure-java-commons is referenced in generated source code, i.e. it must not be test-scoped, only allure-testng should be.
  2. Only allure-java-commons contains aspects and be used as an aspect library, not allure-testng. The latter is simply a normal test dependency.
  3. Neither of the two dependencies need to be weaveDependencies, because they are on the regular test classpath already. When defined as weave dependencies, it means that Allure also sees its own classes and tries to weave into them, if they happen to carry the annotations like @Step.

You want to avoid weaving the same stuff multiple times, a library weaving itself or similar things. After I have cleaned up your project accordingly, I locally can no longer reproduce the problem. Let's see if your CI build or your original project can. See commit https://github.com/afathonih/ajcwovenissue/pull/1/commits/6af5ef42b07d7bbf58cda2b2dc7830b2308f6571.

afathonih commented 1 year ago

@kriegaex Awesome. The build finally pass.

I saw that you restricted the weaver to run only during test-compile. So, I tried it as well on a new branch. With this single change, the the issue actually no longer reproducible. 🎉

I looked back into the failing builds. The weaving during compile part was passing but the test-compile weaving was failing. Probably AspectJ was not expecting a weaved class from the get go (?).

Thank you so much for spending your time on this issue.

kriegaex commented 1 year ago

I saw that you restricted the weaver to run only during test-compile.

Yes, my commit message describes that.

So, I tried it as well on a new branch. With this single change, the the issue actually no longer reproducible. 🎉

Anyway, I fixed other things in additional commits, which were not purely cosmetical in nature but some of them real bugs. I strongly suggest to also fix those in your own project. So please, do read my commit messages and evaluate the changes, both in the dependency list and in the AJ Maven plugin configuration. That a single change makes the problem disappear in your project, does not mean that you are not going to run in other problems subsequently.

The weaving during compile part was passing but the test-compile weaving was failing. Probably AspectJ was not expecting a weaved class from the get go (?).

Re-weaving (or in another usage mod over-weaving) already woven classes should work under normal circumstances, and there might be something that needs improvement in the AJ weaver. But in this case, it was easier to just fix the broken build and not run into the problems in the first place.