[reactive-pg-client] NullPointerException in PgPoolRecorder.java

rgmz commented 2 years ago

Describe the bug

When migrating from 2.9.2.Final to 2.10.1.Final the tests for one of my applications started failing due to a NullPointerException:

java.lang.RuntimeException: Failed to start quarkus
    at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)
    at io.quarkus.runtime.Application.start(Application.java:101)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at io.quarkus.runner.bootstrap.StartupActionImpl.run(StartupActionImpl.java:237)
    at io.quarkus.test.junit.QuarkusTestExtension.doJavaStart(QuarkusTestExtension.java:250)
    at io.quarkus.test.junit.QuarkusTestExtension.ensureStarted(QuarkusTestExtension.java:609)
    at io.quarkus.test.junit.QuarkusTestExtension.beforeAll(QuarkusTestExtension.java:647)
    ... 35 more
Caused by: java.lang.NullPointerException
    at io.quarkus.reactive.pg.client.runtime.PgPoolRecorder.toPgConnectOptions(PgPoolRecorder.java:193)
    at io.quarkus.reactive.pg.client.runtime.PgPoolRecorder.initialize(PgPoolRecorder.java:68)
    at io.quarkus.reactive.pg.client.runtime.PgPoolRecorder.configurePgPool(PgPoolRecorder.java:47)
    at io.quarkus.deployment.steps.ReactivePgClientProcessor$build897843755.deploy_0(Unknown Source)
    at io.quarkus.deployment.steps.ReactivePgClientProcessor$build897843755.deploy(Unknown Source)
    ... 51 more

The cause seems to be that dataSourceReactiveRuntimeConfig.additionalProperties can be null, so when it hits this line it fails: https://github.com/quarkusio/quarkus/blob/3ad7cd0ec6a2297ccda5759b5acb22741d147d37/extensions/reactive-pg-client/runtime/src/main/java/io/quarkus/reactive/pg/client/runtime/PgPoolRecorder.java#L193

Expected behavior

The application starts up successfully.

Actual behavior

The application fails.

How to Reproduce?

I cannot share a reproducer at this time because the code belongs to my company and it would be difficult to recreate.

However, based on my troubleshooting the issue seems to triggered with this configuration:

"%test":
  quarkus:
    datasource:
      app:
        db-kind: postgresql
        devservices:
          enabled: true
          image-name: postgres:14
        username: "quarkus"
        password: "quarkus"
      migration:
        db-kind: postgresql
        # This is an awkward workaround to prevent multiple instances of DevServices running.
        jdbc:
          url: "${quarkus.datasource.\"app\".jdbc.url}"
        username: "quarkus"
        password: "quarkus"

     flyway:
       migration:
         # settings

Aside: Having two separate datasources is an awkward workaround run Flyway migrations as a separate user from the standard application operations. For tests it doesn't matter.

Ideally, it'd be possible to specify different credentials in the URL for reactive and JDBC, or make it so reactive/jdbc can be disabled per data source. I'll probably make a separate issue about this...)

When PgPoolRecorder.configurePgPool is called for the "app" datasource, dataSourceReactiveRuntimeConfig.additionalProperties is empty. However, when it's called for the migration datasource,dataSourceReactiveRuntimeConfig.additionalProperties is null.

The straightforward solution would be to surround this with a null check. But the fact that it's null in the first place may be a bug.

Edit: if it's expected that additionalProperties can be null, the null check would need to be added to all pool recorders. https://github.com/quarkusio/quarkus/commit/dccb8d968a1ba9e387439de187e91aca7d2053c8

Output of `uname -a` or `ver`

No response

Output of `java -version`

11.0.12

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.10.1.Final

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

3.8.5

Additional information

No response

quarkus-bot[bot] commented 2 years ago

/cc @tsegismont, @vietj

gsmet commented 2 years ago

Ideally, it'd be possible to specify different credentials in the URL for reactive and JDBC, or make it so reactive/jdbc can be disabled per data source. I'll probably make a separate issue about this...)

You can disable/enable reactive and JDBC per datasource with:

quarkus.datasource.migration.jdbc=true/false
quarkus.datasource.migration.reactive=true/false

You can also disable the devservices for a particular datasource:

quarkus.datasource.migration.devservices.enabled=false

rgmz commented 2 years ago

You can disable/enable reactive and JDBC per datasource with:
quarkus.datasource.migration.jdbc=true/false
quarkus.datasource.migration.reactive=true/false

Good to know. I must have missed this when browsing the configuration reference because I was looking for a .enabled property.

fedinskiy commented 1 year ago

@gsmet it looks like 2.7 is not affected[1] by this bug(and OPs description supports this). Was label 2.7.7.Final added by mistake or "just in case"?

[1]

git clone git@github.com:fedinskiy/reproducer.git -b reproducer/npe-datasource
cd reproducer
mvn quarkus:dev -Dquarkus.platform.version=2.10.1.Final -Dquarkus.platform.group-id=io.quarkus  # fails 
mvn quarkus:dev -Dquarkus.platform.version=2.7.6.Final -Dquarkus.platform.group-id=io.quarkus # works

quarkusio / quarkus