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

Fix NPE when the subprotocol of websocket is null #28268

Merged
merged 1 commit into from
Sep 29, 2022
Merged

Conversation

yesunch
Copy link
Contributor

@yesunch yesunch commented Sep 28, 2022

This PR fixes this issue related to a NPE in SmallRyeGraphQLOverWebSocketHandler.java when connecting with websocat

The fix is tested against the reproducer provided in the issue

Copy link
Member

@phillip-kruger phillip-kruger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@quarkus-bot

This comment has been minimized.

@gsmet gsmet changed the title Fix NPE when the subprotocal of websocket is null Fix NPE when the subprotocol of websocket is null Sep 29, 2022
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed a typo in subprotocal in the warn message and also made a few more changes, the most "important" one being avoiding a string concatenation for a debug message that will probably never be displayed.

All force pushed to your branch so let's wait for CI and merge.

@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 29, 2022
@yesunch
Copy link
Contributor Author

yesunch commented Sep 29, 2022

Ok thanks for the fix!

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 29, 2022

Failing Jobs - Building 47350db

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

@gsmet gsmet merged commit a472f55 into quarkusio:main Sep 29, 2022
@gsmet
Copy link
Member

gsmet commented Sep 29, 2022

And merged, thanks!

@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Sep 29, 2022
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 29, 2022
@gsmet gsmet modified the milestones: 2.14 - main, 2.13.1.Final Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

quarkus-smallrye-graphql: NPE when connecting with websocat
3 participants