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

convert SocketChannelEndpoint to SocketChannel #42419

Merged

Conversation

escherize
Copy link
Contributor

@escherize escherize commented May 8, 2024

Fixes #13771

During SSL, the getTransport method was returning a type that we didn't expect: SocketChannelEndpoint, and that type was unable to be cast into a SocketChannel. Calling .getChannel on the SocketChannelEndpoint returns a SocketChannel, so that is what we do.

@escherize escherize requested a review from camsaul as a code owner May 8, 2024 19:20
@metabase-bot metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label May 8, 2024
@paoliniluis paoliniluis added the backport Automatically create PR on current release branch on merge label May 8, 2024
Copy link

replay-io bot commented May 8, 2024

Status Complete ↗︎
Commit 8bc0432
Results
⚠️ 7 Flaky
2474 Passed

Comment on lines +154 to +160
(defn log-unexpected-transport!
"Log an error when an unexpected transport is encountered."
[transport]
(let [transport-type (type transport)]
(when-not (contains? @*reported-types transport-type)
(log/errorf "Unexpected transport type encountered in `canceled?`: %s" transport-type))
(swap! *reported-types conj transport-type)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We just want to log this error once per unexpected type

Copy link
Contributor

@johnswanson johnswanson left a comment

Choose a reason for hiding this comment

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

Nice, looks good!

@escherize escherize requested a review from johnswanson May 8, 2024 20:29
@escherize escherize enabled auto-merge (squash) May 8, 2024 20:51
@escherize escherize merged commit 6ca8393 into master May 8, 2024
111 checks passed
@escherize escherize deleted the bcm-convert-socket-channel-endpoints-to-socket-channels branch May 8, 2024 20:52
Copy link

github-actions bot commented May 8, 2024

@escherize Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone?

github-actions bot pushed a commit that referenced this pull request May 8, 2024
* convert `SocketChannelEndpoint` to `SocketChannel`

* Remove fatal logging (used for debugging)

* check for nil channel + log on unknown type

* only log unexpected transport types once
metabase-bot bot added a commit that referenced this pull request May 8, 2024
* convert `SocketChannelEndpoint` to `SocketChannel`

* Remove fatal logging (used for debugging)

* check for nil channel + log on unknown type

* only log unexpected transport types once

Co-authored-by: bryan <bryan.maass@gmail.com>
@dosubot dosubot bot added this to the 0.49.9 milestone May 9, 2024
@sloansparger sloansparger removed this from the 0.49.9 milestone May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/AdminWebapp Admin and Webapp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERROR async.streaming-response when Jetty SSL enabled
5 participants