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

Clarify WebSocketUpgradeFilter.addMapping() method usage #7771

Closed
joakime opened this issue Mar 23, 2022 · 1 comment · Fixed by #7772
Closed

Clarify WebSocketUpgradeFilter.addMapping() method usage #7771

joakime opened this issue Mar 23, 2022 · 1 comment · Fixed by #7772
Assignees

Comments

@joakime
Copy link
Contributor

joakime commented Mar 23, 2022

Originally posted by @lachlan-roberts in #7078 (comment)

@tresf I see from your stacktrace the error is from PrintSocketServer.

It doesn't make much sense to be configuring the mappings before starting as the mappings are cleared on restart, so this change was intentional as without it other use cases are broken. I see that this change does break your usage of WebSocketUpgradeFilter.addMapping(), I think this method should probably be deprecated as well.

@joakime joakime added the Bug For general bugs on Jetty side label Mar 23, 2022
joakime added a commit that referenced this issue Mar 23, 2022
@joakime joakime assigned joakime and unassigned lachlan-roberts Mar 23, 2022
@joakime joakime added Documentation and removed Bug For general bugs on Jetty side labels Mar 23, 2022
@joakime joakime changed the title Deprecate WebSocketUpgradeFilter.addMapping() methods Clarify WebSocketUpgradeFilter.addMapping() method usage Mar 23, 2022
@joakime joakime linked a pull request Mar 23, 2022 that will close this issue
@joakime
Copy link
Contributor Author

joakime commented Mar 23, 2022

Opened PR #7772

joakime added a commit that referenced this issue Mar 23, 2022
joakime added a commit that referenced this issue Mar 24, 2022
@joakime joakime closed this as completed Mar 24, 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 a pull request may close this issue.

2 participants