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

Initial message fix #593

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

akbashev
Copy link
Contributor

@akbashev akbashev commented Jul 16, 2024

Last week I've been checking bidirectional examples with latest Xcode 16 beta 3 and realised it uses HBv2, which is supported only by macOS 14+, and it's not perfect. After downgrading to HBv1—same problem with returning stream until some bytes are sent appeared. Haven't figured out yet why, maybe some tech inside uses URLSession. So, this PR fixes two things:

  • Moved server to Vapor for two reasons: 1) anyway can't use HBv2 now and 2) it could be more familiar to others;
  • Moved back to URLSession for client and added initial message hack.

It's working for now. I'll be checking from time to time status of Swift Server tools, maybe we can improve it in the future.

@@ -20,14 +20,14 @@ let package = Package(
dependencies: [
.package(url: "https://github.com/apple/swift-openapi-generator", from: "1.0.0"),
.package(url: "https://github.com/apple/swift-openapi-runtime", from: "1.2.0"),
.package(url: "https://github.com/swift-server/swift-openapi-async-http-client", from: "1.0.0"),
.package(url: "https://github.com/apple/swift-openapi-urlsession", from: "1.0.0"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine to keep it using AHC? Since it doesn't require the initial message hack?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@czechboy0 we could, it doesn't matter in the end as there something with transport happening (both for vapor and hbv1), so we anyway need initial message

Copy link
Contributor

Choose a reason for hiding this comment

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

there something with transport happening (both for vapor and hbv1)

Can you elaborate what you're seeing? Are you saying even when using AHC, you still don't get the stream established without an initial message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you elaborate what you're seeing? Are you saying even when using AHC, you still don't get the stream established without an initial message?

Yep, exactly. I will try to investigate, but for now think we can just update the example.

Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

@akbashev if you have some time, please investigate where this issue might be, I'd prefer not to switch the example until we have a good understanding of why it doesn't work

@akbashev
Copy link
Contributor Author

@czechboy0 will check, last time I've tried—it was a bit hard, cause it's hard to debug "not happening" events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants