vaadin / quarkus

An extension to Quarkus to support Vaadin Flow
Apache License 2.0
28 stars 3 forks source link

PiT 24.2: generated project does not work with 24.2 (mac and windows) #136

Closed manolo closed 11 months ago

manolo commented 11 months ago

Description of the issue

The generated project does not work when upgrading from 24.1 to 24.2

How to reproduce

# generate project
mvn -ntp -q -B io.quarkus.platform:quarkus-maven-plugin:3.1.1.Final:create -Dextensions=vaadin -DwithCodestart -DprojectGroupId=com.vaadin.starter -DprojectArtifactId=vaadin-quarkus
# change to folder
cd vaadin-quarkus
# upgrade version
perl -pi -e 's/24.1.2/24.2.0.alpha2/' pom.xml
# clean and run (remove .cmd when not in windows)
./mvnw.cmd -ntp -B clean 
./mvnw.cmd -ntp -B quarkus:dev

System

java: openjdk version "17.0.6" 2023-01-17

manolo commented 11 months ago

Error in mac

Caused by: java.lang.reflect.InvocationTargetException
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:119)
    at java.base/java.lang.reflect.Method.invoke(Method.java:578)
    at com.vaadin.flow.server.startup.ClassLoaderAwareServletContainerInitializer.lambda$onStartup$2(ClassLoaderAwareServletContainerInitializer.java:98)
    ... 27 more
Caused by: java.lang.NoClassDefFoundError: com/sun/jna/platform/unix/LibCAPI$size_t$ByReference
    at oshi.hardware.platform.mac.MacCentralProcessor.initProcessorCounts(MacCentralProcessor.java:119)
    at oshi.hardware.common.AbstractCentralProcessor.<init>(AbstractCentralProcessor.java:65)
    at oshi.hardware.platform.mac.MacCentralProcessor.<init>(MacCentralProcessor.java:47)
    at oshi.hardware.platform.mac.MacHardwareAbstractionLayer.createProcessor(MacHardwareAbstractionLayer.java:42)
    at oshi.util.Memoizer$1.get(Memoizer.java:61)
    at oshi.hardware.common.AbstractHardwareAbstractionLayer.getProcessor(AbstractHardwareAbstractionLayer.java:48)
    at com.vaadin.pro.licensechecker.MachineId.getComputerId(MachineId.java:34)
    at com.vaadin.pro.licensechecker.MachineId.get(MachineId.java:19)
    at com.vaadin.base.devserver.stats.DevModeUsageStatistics.lambda$trackGlobalData$0(DevModeUsageStatistics.java:118)
    at com.vaadin.base.devserver.stats.StatisticsStorage.lambda$update$f902d37c$1(StatisticsStorage.java:95)
    at com.vaadin.base.devserver.stats.StatisticsStorage.access(StatisticsStorage.java:79)
    at com.vaadin.base.devserver.stats.StatisticsStorage.update(StatisticsStorage.java:91)
    at com.vaadin.base.devserver.stats.DevModeUsageStatistics.trackGlobalData(DevModeUsageStatistics.java:105)
    at com.vaadin.base.devserver.stats.DevModeUsageStatistics.lambda$init$d066c0ae$1(DevModeUsageStatistics.java:97)
    at com.vaadin.base.devserver.stats.StatisticsStorage.access(StatisticsStorage.java:79)
    at com.vaadin.base.devserver.stats.DevModeUsageStatistics.init(DevModeUsageStatistics.java:92)
    at com.vaadin.base.devserver.startup.DevModeInitializer.initDevModeHandler(DevModeInitializer.java:203)
    at com.vaadin.base.devserver.DevModeHandlerManagerImpl.initDevModeHandler(DevModeHandlerManagerImpl.java:96)
    at com.vaadin.base.devserver.startup.DevModeStartupListener.initialize(DevModeStartupListener.java:76)
    at com.vaadin.flow.server.startup.VaadinServletContextStartupInitializer.process(VaadinServletContextStartupInitializer.java:42)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)

Error in windows

Caused by: java.lang.NoSuchMethodError: 'void com.sun.jna.Memory.close()'
        at oshi.util.Util.freeMemory(Util.java:83)
        at oshi.jna.Struct$CloseableSystemInfo.close(Struct.java:166)
        at oshi.hardware.platform.windows.WindowsCentralProcessor.queryProcessorId(WindowsCentralProcessor.java:129)
        at oshi.util.Memoizer$1.get(Memoizer.java:61)
        at oshi.hardware.common.AbstractCentralProcessor.getProcessorIdentifier(AbstractCentralProcessor.java:107)
        at com.vaadin.pro.licensechecker.MachineId.getComputerId(MachineId.java:35)
        at com.vaadin.pro.licensechecker.MachineId.get(MachineId.java:19)
        at com.vaadin.base.devserver.stats.DevModeUsageStatistics.lambda$trackGlobalData$0(DevModeUsageStatistics.java:118)
        at com.vaadin.base.devserver.stats.StatisticsStorage.lambda$update$f902d37c$1(StatisticsStorage.java:95)
        at com.vaadin.base.devserver.stats.StatisticsStorage.access(StatisticsStorage.java:79)
manolo commented 11 months ago

Changing the order of quarkus and vaadin boms in pom.xml fixes the problem. But we cannot control this order in the quarkus.io generator.

But now the question is why the project works in 24.1.2 and fails with 24.2, when jna versions has not changed? maybe @Artur- knows ?

manolo commented 11 months ago

Seems that the culprit is this PR

globalData.setValue(StatisticsConstants.FIELD_MACHINE_ID, MachineId.get());
mvysny commented 11 months ago

Excellent catch @manolo , that's definitely the culprit. This code was added just recently, as an effort to have a more detailed statistics on the development environment (side note: this bug only affects dev env; no stats are collected in production therefore this exception shouldn't happen in production).

Having a working dev env with Quarkus is much more important than the stats; a quick solution is therefore necessary. I'd simply propose to log "-" as machine ID, in case of any exception. That should fix this issue, and will give us time to start devising a more proper fix.

Alternatively, we could detect the presence of Quarkus and disable MachineID statistics collection and log something like "unknown-quarkus". However, I don't think this is a good solution, since the problem is essentially a classpath conflict, and not directly linked to the Quarkus itself.

Since the PR was done as a recent effort to improve statistics, I'm adding @Artur- to the loop.

mcollovati commented 11 months ago

I think there's no need for Quarkus detection, as the issue can be resolved by swapping the boms. BTW, this clash may not be limited to Quarkus, but it could, for example, also appear with Spring Boot if one day they will pin jna or oshi version on their bom.

Anyway, the Quarkus add-on Know Issues documentations should be updated, providing the instructions on how to solve the problem.

manolo commented 11 months ago

But still, I guess it's worth to catch the exception, and don't break DX with a weird exception. Most ppl is probably lazy to read documentation.

mcollovati commented 11 months ago

I propose to proceed adding the error handling around the machine ID retrieval, and then we can revert the change in the future if we are not satisfied with it. If you agree, I can go ahead with the change in Flow

mcollovati commented 11 months ago

As a side note, MachineId.get() wraps the execution in a try/catch block, silently failing and returning blank if there's an exception. However, in the mentioned cases, the code is throwing an Error so it is not caught by the try/catch block. This is good because otherwise, if we suppress the error, we will lose an important indicator that something is going wrong.

So, another solution may be to change the MachineId.get() method in the license-checker to catch Error and log the cause (warn message + stacktrace at debug level?) and return blank

mcollovati commented 11 months ago

But for the moment, we can add a temporary fix on dev-server