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

[Quarkus 3] Pact-Consumer-Test failing in XStream when injecting Pact Mockserver #34550

Closed
lostiniceland opened this issue Jul 5, 2023 · 9 comments
Labels
area/testing kind/bug Something isn't working

Comments

@lostiniceland
Copy link
Contributor

Describe the bug

Migrating from Quarkus 2 to 3 we encountered a blocking issue with Quarkus 3 in combination with Pact-Jvm.

All Consumer-Tests run into a Stackoverflow-Error when Pact is doing some serialization using XStream.

Please see attached demo-project, containing the same test in Quarkus 2 and 3. Running mvn verify will pass Quarkus 2 and fail 3.

The only difference which might be interesting during investigation is that we had to add tons of add-opens directives to the surefire-plugin in the Quarkus 3 case, which wasnt necessary in 2. Since they run the same JVM and the same Pact version, this behaviour is quite odd.
Also note that the surefire-plugin has to be configured not to use the System-Classloader according to the Pact-JVM documentation.

quarkus-pact-issue.tar.gz

Fun fact: running a Pact-Provider test works just fine.

Expected behavior

Test runs to completion

Actual behavior

Stackoverflow occurs

[ERROR] demo.Quarkus3FooClientPactTest.testContract  Time elapsed: 1.125 s  <<< ERROR!
com.thoughtworks.xstream.converters.ConversionException: 
Failed calling method
---- Debugging information ----
message             : Failed calling method
cause-exception     : java.lang.StackOverflowError
cause-message       : null
method              : java.util.ArrayDeque.writeObject()
-------------------------------
	at com.thoughtworks.xstream.core.util.SerializationMembers.callWriteObject(SerializationMembers.java:161)
	at com.thoughtworks.xstream.converters.reflection.SerializableConverter.doMarshal(SerializableConverter.java:257)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshal(AbstractReflectionConverter.java:90)
	at com.thoughtworks.xstream.core.AbstractReferenceMarshaller.convert(AbstractReferenceMarshaller.java:68)
	at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:59)
	at com.thoughtworks.xstream.core.AbstractReferenceMarshaller$1.convertAnother(AbstractReferenceMarshaller.java:83)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshallField(AbstractReflectionConverter.java:270)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter$2.writeField(AbstractReflectionConverter.java:174)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.doMarshal(AbstractReflectionConverter.java:262)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshal(AbstractReflectionConverter.java:90)
	at com.thoughtworks.xstream.core.AbstractReferenceMarshaller.convert(AbstractReferenceMarshaller.java:68)
	at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:59)
	at com.thoughtworks.xstream.core.AbstractReferenceMarshaller$1.convertAnother(AbstractReferenceMarshaller.java:83)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshallField(AbstractReflectionConverter.java:270)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter$2.writeField(AbstractReflectionConverter.java:174)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.doMarshal(AbstractReflectionConverter.java:262)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshal(AbstractReflectionConverter.java:90)
	at com.thoughtworks.xstream.core.AbstractReferenceMarshaller.convert(AbstractReferenceMarshaller.java:68)
	at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:59)
	at com.thoughtworks.xstream.core.AbstractReferenceMarshaller$1.convertAnother(AbstractReferenceMarshaller.java:83)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshallField(AbstractReflectionConverter.java:270)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter$2.writeField(AbstractReflectionConverter.java:174)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.doMarshal(AbstractReflectionConverter.java:262)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshal(AbstractReflectionConverter.java:90)
	at com.thoughtworks.xstream.core.AbstractReferenceMarshaller.convert(AbstractReferenceMarshaller.java:68)
	at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:59)
	at com.thoughtworks.xstream.core.AbstractReferenceMarshaller$1.convertAnother(AbstractReferenceMarshaller.java:83)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshallField(AbstractReflectionConverter.java:270)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter$2.writeField(AbstractReflectionConverter.java:174)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.doMarshal(AbstractReflectionConverter.java:262)
	at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshal(AbstractReflectionConverter.java:90)
	at com.thoughtworks.xstream.core.AbstractReferenceMarshaller.convert(AbstractReferenceMarshaller.java:68)
	at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:59)
...

How to Reproduce?

Extract provided sample

Run mvn verify

Output of uname -a or ver

No response

Output of java -version

17.0.1

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.2.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Maven 3.8.6

Additional information

There are already some issues involving Quarkus-Pact interop (see for example here) which seems to be why there is some working going on in the relatively new Quarkiverse-Pact-Extension. Unfortunately using this extension still doesnt solve the issue (the consumer-extension isnt currently doing much except providing the dependencies).

The Pact-JVM implementation is kinda strange and maybe the root lies there, but since the sample works in Quarkus 2 I've decided to place the issue here.

@lostiniceland lostiniceland added the kind/bug Something isn't working label Jul 5, 2023
@markusdlugi
Copy link
Contributor

Hi @lostiniceland. Just stumbled upon this issue by accident, because we noticed the same problem in our project during our Quarkus 3 migration efforts :)

For us, the error vanished after removing the injected MockServer. I can also confirm that injecting the MockServer still worked in Quarkus 2. So maybe you can try that as well. See also this issue in the quarkus-pact repo:

quarkiverse/quarkus-pact#73

@lostiniceland
Copy link
Contributor Author

lostiniceland commented Jul 6, 2023

@markusdlugi Thanks for sharing. I was hoping for some kind of workaround.

I love your idea having Wiremock as a proxy because log-output of Pacts mockserver is a disaster

@geoand
Copy link
Contributor

geoand commented Jul 6, 2023

cc @holly-cummins

@holly-cummins
Copy link
Contributor

Thanks for reporting, and for the reproducer, @lostiniceland. I think this is closely related to quarkiverse/quarkus-pact#73. Fixing this needs some changes to the pact extension, and also to core Quarkus, but no fixes can happen until some Quarkus dependencies are released. However, I notice that the Pact extensions are being added 'loose', rather than via the Pact extension. Pact extension is the recommended way of using Pact with Quarkus, rather than the raw libraries. So I'd suggested switching to the extension as a first action. I think things will still be broken, because of #73, but you'll be on the path where you'll get fixes.

Here's a few extra pieces of context:

  • I know it seems like the Pact extension isn't doing much, and that's kind of true, and it's kind of not-true. In the long term, the plans are that the Pact extension will allow a lot of the DSL boilerplate involved in defining a contract to be removed, because we can start with a @restclient and read some expectations from that, like what already happens for Mockito today. Also in plan is auto-configuring the port and a PactBroker dev service. All of that work is blocked by JUnit classloading restrictions. In the short term, the Pact extension does a couple of things

    • Without the Pact extension Pact tests all die horribly if continuous testing is used, but the Pact extension makes this work (with some dependency-scope workarounds needed in Quarkus 2)
    • Pact uses Kotlin under the covers, and the Kotlin classloading changed a lot between Quarkus 2 and 3 (partly to improve Pact support, see Don't load Kotlin parent first #29697). That means how the Pact dependencies are included had to change between Quarkus 2 and 3. So far, for the consumer we were able to remove the parentfirst classloading, and for the provider we haven't been able to. So that's why providers and consumers will have some different behaviours. As long as you've got the right versions of the extension for your Quarkus level, the extension abstracts away all the different config. (Why did we want to get away from parent first classloading? It blocks lots of Quarkus developer joy, and it can also ripple through in a viral way and mean other dependencies get loaded parent first, so that they stop working, and ... it's just better not to use it unless absolutely necessary.)
    • The extension has lots of tests, which allow us to make sure Pact works with Quarkus. For example, the MockServer regression was caught by extension tests, it just ... wasn't fixed, because fixing it was quite hard.
  • The xstream stack traces are a side effect of some classloader tricks that Quarkus has to do. The tricks no longer work on Java 17 and above, which is why you get a sad stack trace. Quarkus changes the bytecode of application and test classes, so that it can move work to build time and get rid of boilerplate and all sorts of other good things. In continuous testing/dev mode, the modified classes are loaded by a special classloader. However, JUnit loads everything from the default classloader. What the Quarkus test support does is 'hop' between classloaders. JUnit gives us a class loaded in the default classloader, and we then swap it for one loaded with our classloader. Moving instances between classloaders is hard. The way Quarkus does it is to serialize the instance, and then deserialize it in the new classloader. Unfortunately, from Java 17 on, the JVM blocks that kind of private method access.

  • With JUnit 5.10, we should be able to just tell JUnit what classloader to use. JUnit 5.10-M1 is released, so I'll do the experiments soon to confirm it actually does all the things we hope it will. There's more discussion of this in #22611.

  • I have revised my opinion of the priority of the MockServer defect! :)

You might be able to work around the original xtream stack trace by either

  • Running with a JVM below 16, although obviously that's incredibly annoying
  • As suggested in Unable to run my project on Java 16 x-stream/xstream#260, add some -add-opens. I know you already have the -add-opens, but not the Quarkus extension. The combination of the two might be more fruitful.

@holomekc
Copy link
Contributor

holomekc commented Jul 6, 2023

@holly-cummins
thx for the detailed information!

The approach in our wiremock / pact setup is, that we have a Quarkus test resource for Wiremock with a custom annotation, which makes it easy to inject Wiremock into tests etc. In a base test class, we do exactly that and provide a method, which accepts the MockServer and creates the proxy config for Wiremock using the MockServer url. Then we just need to call this method. In theory this setup can be achieved wherever the MockServer is available then.

We will check if we can share something, without sharing internal information.

@lostiniceland
Copy link
Contributor Author

@holly-cummins Thanks as well

We also decided that moving to the pact-extension would be the best solution since we expected that in case there is some magic to be done it will happen there and not in pact itself. So we implemented the workaround with the extension by now.

The demo was created without the extension because I was not sure how quarkiverse-extensions are treated within the Quarkus project with regards to issues and since both created the same result I placed the issue here rather than in the extension.

@holly-cummins
Copy link
Contributor

@lostiniceland, do you mind editing the issue title to make it more clear the symptom is in Xstream and what exposes the issue is MockServer? That makes it easier for others who might have similar issues to find the work item.

Also, I've just seen that JUnit 5.10-RC1 has dropped, so we're getting closer to getting that in.

@holomekc
Copy link
Contributor

holomekc commented Jul 6, 2023

WireMock demo and workaround for the issue here: holly-cummins/pact-quarkus-sweater-demo#122

@lostiniceland lostiniceland changed the title Pact-Consumer-Test not working with Quarkus 3 [Quarkus 3] Pact-Consumer-Test failing in XStream when injecting Pact Mockserver Jul 10, 2023
@holly-cummins
Copy link
Contributor

This is closed by #40749 , and indirectly, #40601

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants