Skip to content

remove false comment about try-with-resources statement in StompSubProtocolHandler #35053

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

remove false comment about try-with-resources statement in StompSubProtocolHandler #35053

wants to merge 3 commits into from
+173 −2

Conversation

Pankraz76
Copy link

No description provided.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 14, 2025
@Pankraz76 Pankraz76 force-pushed the fix-StompSubProtocolHandler branch from b0f0f9a to 547a0dc Compare June 14, 2025 09:09
@Pankraz76 Pankraz76 marked this pull request as ready for review June 14, 2025 09:09
Pankraz76
Pankraz76 commented Jun 14, 2025
View reviewed changes
...ebsocket/src/main/java/org/springframework/web/socket/messaging/StompSubProtocolHandler.java
@@ -399,8 +399,6 @@ private void sendErrorMessage(WebSocketSession session, Throwable error) {
headerAccessor.setMessage(error.getMessage());

byte[] bytes = this.stompEncoder.encode(headerAccessor.getMessageHeaders(), EMPTY_PAYLOAD);
Copy link
Author

Choose a reason for hiding this comment

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

primitive data type is not implementing interface autocloseable, seems impossible to use try-with-resources, right?.

not possible in L:413 ether, so idk if we still need this comment, as misleading.

image

@Pankraz76 Pankraz76 mentioned this pull request Jun 14, 2025
Closed
@bclozel
Copy link
Member

bclozel commented Jun 14, 2025

I don't understand the goal of this PR. The WebSocketSession is not a primitive type and the comment is very much on point. We want IO exceptions to be thrown during writing, but we want to silently swallow the ones happening during close. Again, this link should tell you more about this pattern.

@bclozel bclozel closed this Jun 14, 2025
@bclozel bclozel added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 14, 2025
@bclozel bclozel mentioned this pull request Jun 14, 2025
Closed
@Pankraz76
Copy link
Author

can we merge test then at least to improve cover?

@Pankraz76 Pankraz76 mentioned this pull request Jun 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Pankraz76 @bclozel @spring-projects-issues