GraphQL Field level context and error event on blocking fixes

[phillip-kruger]

Fix #27628
Fix smallrye/smallrye-graphql#1530

see https://github.com/smallrye/smallrye-graphql/releases/tag/1.7.1

Signed-off-by: Phillip Kruger [email protected]

[gsmet]

@jmartisk @phillip-kruger this update is missing the Jakarta counterpart. Could you fix it and make sure it doesn't happen again? Thanks.

Also, as reported elsewhere, I suspect this patch is triggering the context leaks again. See #27742 (comment) .

[gsmet]

Hmmm, the update is there but the tests are failing with a weird class error. I will report back here when I know more.

[gsmet]

Yeah, so sorry about the confusion. Apparently, it's still the BeanManager#fireEvent thing being an issue. Which is weird given I can see the fix in the released branches. I'm trying to understand what's going on.

[gsmet]

Oh, my, I think I will go back to bed. Doesn't look like a good day... I was looking in the history to better understand things and ended up working on an old report...

So the current issue at hand is a NPE when we try to index a GraphQL class.

This is because we have both Jandex versions in the classpath when GraphQL is around and the new Jandex is taking precedence, returning null when a class is indexed:

[INFO] |  |  \- io.smallrye:jandex:jar:3.0.0:compile
[INFO] |  |  \- org.jboss:jandex:jar:2.4.3.Final:test

@Ladicek I wonder if we should push the fix to IndexingUtil (classInfo = indexer.index(stream);) right now as a quick workaround. We could add exclusions but you would have to remove them soon so I don't think that would be ideal.

Asking because having the report full of unrelated failures makes things a bit harder for me.

[Ladicek]

Humm, did we already upgrade to SmallRye GraphQL that depends on Jandex 3? I expected the Jandex 3 upgrade would bump SmallRye GraphQL (and OpenAPI)...

[Ladicek]

Ah it's the Jakarta variant of SmallRye GraphQL that already depends on Jandex 3. Humpf...

[gsmet]

@Ladicek yeah, exactly. I don't think it's worth spending much time on this given we will merge the Jandex upgrade on Sep 14th or around that. But if the IndexingUtil fix makes it work, maybe it's worth pushing just that (provided it's not risky)?

[Ladicek]

In my Jandex 3 branch, I had to change IndexingUtil to use a Jandex 3 API, so that can't be directly ported to the Jakarta branch. However, looking at the SmallRye GraphQL changes I did to upgrade to Jandex 3, it should still be compatible at runtime with Jandex 2.4.3. So adding a dependency exclusion would likely be easiest at the moment.

[gsmet]

OK, I'll think about it. Not sure it's worth it. If I prepare a PR with an exclusion, I'll ping you there so you can adjust your Jandex 3 PR to remove it.