Update release.yml #1
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
### Motivation: SPM has built in functionality to check the API of modules against a target git treeish. We can use this to simplify our `check_no_api_breakages.sh` script. Closes apple/swift-nio#1239 ### Modifications: This PR, exchanges the direct calls to Swift's API checker with the new SPM `diagnose-api-breaking-changes` tool. This allows us to get rid of the manual module parsing, build invocations and result comparisons. ### Result: We are now using SPMs `diagnose-api-breaking-changes` to check for breaking changes.
Motivation The RequestBag intermediates between two different threads. This means it can get requests that were reasonable when they were made but have been superseded with newer information since then. These generally have to be tolerated. Unfortunately if we received a request to resume the request body stream _after_ the need for that stream has been invalidated, we could hit a crash. That's unnecessary, and we should tolerate it better. Modifications Tolerated receiving requests to resume body streaming in the finished state. Result Fewer crashes Fixes #576
Same fix for HTTP/1 that landed for HTTP/2 in #558. ### Motivation `HTTP1ClientChannelHandler` currently does not tolerate immediate write errors. ### Changes Make `HTTP1ClientChannelHandler` resilient to failing writes. ### Result Less crashes in AHC HTTP/1.
### Motivation Currently, we don’t consume the response body of redirect responses. Because of this requests, that receive a redirect response with response body, may hang indefinitely. ### Changes - Consume redirect response body if less than 3kb - Cancel redirect response if larger than 3kb ### Result Redirect responses are consumed. Fixes #574
As outlined in a [Swift forums post in November ’21](https://forums.swift.org/t/swiftnio-swift-version-support/53232), SwiftNIO will only support the latest non-patch Swift release and the 2 immediately prior non-patch versions. - drop support for Swift 5.2 and 5.3. - update CI for Swift 5.4 to run on bionic instead of focal to ensure that we still test bionic.
This adds a shortDescription property to HTTPClientError which provides a short description of the error without associated values.
### Motivation Today `didSendRequestPart` is called after a request body part has been passed to the executor. However, this does not mean that the write hit the socket. Users may depend on this behavior to implement back-pressure. For this reason, we should only call this `didSendRequestPart` once the write was successful. ### Modification Pass a promise to the actual channel write and only call the delegate once that promise succeeds. ### Result The delegate method `didSendRequestPart` is only called after the write was successful. Fixes #565. Co-authored-by: Fabian Fett <[email protected]>
Motivation Currently error reporting with NIO Transport Services is often sub-par. This occurs because the Network.framework connections may enter the waiting state until the network connectivity state changes. We were not watching for the user event that contains the error in that state, so if we timed out in that state we'd just give a generic timeout error, instead of telling the user anything more detailed. Additionally, several of our tests assume that failure will be fast, but in NIO Transport Services we will enter that .waiting state. This is reasonable, as changed network connections may make a connection that was not succeeding suddenly viable. However, it's inconvenient for testing, where we're mostly interested in confirming that the error path works as expected. Modifications - Add an observer of the WaitingForConnectivity event that records it into our state machine for later reporting. - Add support for disabling waiting for connectivity for testing purposes. - Add annotations to several tests to stop them waiting for connectivity. Results Faster tests, better coverage, better errors for our users. Co-authored-by: David Nadoba <[email protected]>
Motivation It's totally acceptable for a HTTP server to respond before a request upload has completed. If the response is an error, we should abort the upload (and we do), but if the response is a 2xx we should probably just finish the upload. In this case it turns out we'll actually hit a crash when we attempt to deliver an empty body message. his is no good! Once that bug was fixed it revealed another: while we'd attempted to account for this case, we hadn't tested it, and so it turns out that shutdown would hang. As a result, I've also cleaned that up. Modifications - Tolerate empty circular buffers of bytes when streaming an upload. - Notify the connection that the task is complete when we're done. Result Fewer crashes and hangs.
…testConnectTimeout()` (#592) * Use a local TCP server that doesn’t accept connections on macOS for `testConnectTimeout()` * fix linting
Motivation In some cases, the last thing that happens in a request-response pair is that we send our HTTP/1.1 .end. This can happen when the peer has sent an early response, before we have finished uploading our body. When it does, we need to be diligent about cleaning up our connection state. Unfortunately, there was an edge in the HTTP1ConnectionStateMachine that processed .succeedRequest but that did not transition the state into either .idle or .closing. That was an error, and needed to be fixed. Modifications Transition to .idle when we're returning .succeedRequest(.sendRequestEnd). Result Fewer crashes
Motivation When users stream their bodies they may still want to send Connection: close headers and terminate the connection early. This should work properly. Unfortunately it became clear that we didn't correctly pass the information that the connection needed to be closed. As a result, we'd inappropriately re-use the connection, potentially causing unnecessary HTTP errors. Modifications Signal whether the connection needs to be closed when the final connection action is to send .end. Results We behave better with streaming uploads.
Motivation If we receive an early HTTP response, the last action on a HTTP/1.1 connection is to send the .end message. While we had an error handling path in the code, it wasn't tested, and when executed it would end up leaking the connection by failing to close it _or_ return it to the pool. This patch fixes the issue by appropriately terminating the connection and adding a test. Modifications Add a test Terminate the connection if sendEnd fails Result Fewer connection leaks
we may recieve a posix `ECONNREFUSED` too
Co-authored-by: David Nadoba <[email protected]>
`NIOAtomics` was deprecated in apple/swift-nio#2204 in favor of `swift-atomics` https://github.com/apple/swift-atomics
Motivation Warnings aren't great, and NIOAtomic is deprecated. Modifications Replace the last use of NIOAtomic with ManagedAtomic. Result Fewer warnings Fixes #606
…t is canceled aftewards (#609)
…nc/await API (#608) Co-authored-by: Cory Benfield <[email protected]>
Motivation When receiving certain patterns of response body parts, we can end up recursing almost indefinitely to deliver them to the application. This can lead to crashes, so we might politely describe it as "sub-optimal". Modifications Keep track of our stack depth and avoid creating too many stack frames. Added some unit tests. Result We no longer explode when handling bodies with lots of tiny parts. Co-authored-by: David Nadoba <[email protected]>
Motivation Documentation is nice, and we can help support users by providing useful clear docs. Modifications Add Docc to 5.6 and later builds Make sure symbol references work Add overview docs Result Nice rendering docs
Motivation We should apply the connect timeout to the complete set of connection attempts, rather than the request deadline. This allows users fine-grained control over how long we attempt to connect for. This is also the behaviour of our old-school interface. Modifications - Changed the connect deadline calculation for async/await to match that of the future-based code. - Added a connect timeout test. Result Connect timeouts are properly handled
* Tollerate more data after request body is cancelled * wait is not needed if we shutdown the server first * Remove test that depends on external resources * Remove unused conformance to Equatable * SwiftFormat * run generate_linux_tests.rb * Increase timeout for CI
`HTTPClient.Response` is trivially `Sendable`, let's mark it `Sendable`.
…t` to `Int.max` (#763) ### Motivation: When a user wishes to make the connection pool create as many concurrent connections as possible, a natural way to achieve this would be to set `.max` to the `concurrentHTTP1ConnectionsPerHostSoftLimit` property. ```swift HTTPClient.Configuration().connectionPool = .init( idleTimeout: .hours(1), concurrentHTTP1ConnectionsPerHostSoftLimit: .max ) ``` The `concurrentHTTP1ConnectionsPerHostSoftLimit` property is of type `Int`. Setting it to `Int.max` leads to `Int.max` being passed as an argument to `Array`s `.reserveCapacity(_:)` method, causing an OOM issue. Addresses Github Issue #751 ### Modifications: Capped the argument to `self.connections.reserveCapacity(_:)` to 1024 in `HTTPConnectionPool.HTTP1Connections` ### Result: Users can now set the `concurrentHTTP1ConnectionsPerHostSoftLimit` property to `.max` without causing an OOM issue.
Since most of the servers now conform to http2, the change here updates the behaviour of assuming the connection to be http2 and not http1 by default. It will migrate to http1 if the server only supports http1. One can set the `httpVersion` in `ClientConfiguration` to `.http1Only` which will start with http1 instead of http2. Additional Changes: - Fixed an off by one error in the maximum additional general purpose connection check - Updated tests --------- Co-authored-by: Ayush Garg <[email protected]> Co-authored-by: David Nadoba <[email protected]> Co-authored-by: Fabian Fett <[email protected]>
Motivation: Would like to use github to autogenerate release notes. Modifications: Bring over configuration consistent with NIO and other popular repos. Result: Once labels are synchronised release notes can be generated by github
…he http request (#768) ### Motivation If the channel's writability changed to false just before we finished a request, we currently run into a precondition. ### Changes - Remove the precondition and handle the case appropiatly ### Result A crash less.
The Darwin module is slowly being split up, and as it gets further along, it will stop importing some of the split-out modules like the one for locale.h that provides newlocale() and other locale API. However, there's a wrinkle that on platforms with xlocale, it's xlocale.h that provides most of the POSIX locale.h functions and not locale.h, so prefer the xlocale module when available.
This PR adds .git extensions to the Swift Package Manager dependencies that didn't include them. For me, this resolves issues that I have had with an error produced by Xcode when updating to the latest package versions if I'm editing the project which depends on AHC in an Xcode Workspace. <img width="389" alt="image" src="https://github.com/user-attachments/assets/24c3a5de-b555-4da8-a2fa-a4673fecff44"> The complete error is: `github.com: https://github.com/apple/swift-algorithms: The repository could not be found. Make sure a valid repository exists at the specified location and try again.` Based on conversations in the Vapor Discord server, adding these extensions "shouldn't" make a difference to the dependency resolution done by swift package manager, however adding them resolves the error. 🤷 Co-authored-by: Franz Busch <[email protected]>
Multipath TCP (MPTCP) is a TCP extension allowing to enhance the reliability of the network by using multiple interfaces. This extension provides a seamless handover between interfaces in case of deterioration of the connection on the original one. In the context of iOS and Mac OS X, it could be really interesting to leverage the capabilities of MPTCP as they could benefit from their multiple interfaces (ethernet + Wi-fi for Mac OS X, Wi-fi + cellular for iOS). This contribution introduces patches to HTTPClient.Configuration and establishment of the Bootstraps. A supplementary field "enableMultipath" was added to the configuration, allowing to request the use of MPTCP. This flag is then used when creating the channels to configure the client. Note that in the future, it might also be potentially interesting to offer more precise configuration options for MPTCP on MacOS, as the Network framework allows also to select a type of service, instead of just offering the option to create MPTCP connections. Currently, when enabling MPTCP, only the Handover mode is used. --------- Co-authored-by: Cory Benfield <[email protected]>
…Handler (#772) ### Motivation: A performance test executing 100,000 sequential requests against a simple [`NIOHTTP1Server`](https://github.com/apple/swift-nio/blob/main/Sources/NIOHTTP1Server/README.md) revealed that 7% of total run time is spent in the setter of the `request` property in `HTTP1ClientChannelHandler` (GitHub Issue #754). The poor performance comes from [processing the string interpolation `"\(self.eventLoop)"`](https://github.com/swift-server/async-http-client/blob/6df8e1c17e68f0f93de2443b8c8cafca9ddcc89a/Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ClientChannelHandler.swift#L39C17-L39C75) which under the hood calls a computed property. This problem can entirely be avoided by storing `eventLoop.description` when initializing `HTTP1ClientChannelHandler`, and using that stored value in `request`'s setter, rather than computing the property each time. ### Modifications: - Created a new property `let eventLoopDescription: Logger.MetadataValue` in `HTTP1ClientChannelHandler` that stores the description of the `eventLoop` argument that is passed into the initializer. - Replaced the string interpolation `"\(self.eventLoop)"` in `request`'s setter with `self.eventLoopDescription`. ### Result: `HTTP1ClientChannelHandler.eventLoop`'s `description` property is cached upon initialization rather than being computed each time in the `request` property's setter. --------- Co-authored-by: Cory Benfield <[email protected]>
`Foundation.URL` has various behavior changes in Swift 6 to better match RFC 3986 which impact AHC. In particular it now no longer strips the square brackets in IPv6 hosts which are not tolerated by `inet_pton` so these must be manually stripped. swiftlang/swift-foundation#957 swiftlang/swift-foundation#958 swiftlang/swift-foundation#962
Motivation: As an HTTP library, async-http-client should have authentication support. Modifications: This adds a `setBasicAuth()` method to both HTTPClientRequest and `HTTPClient.Request` and their related unit tests. Result: Library users will be able to leverage this method to use basic authentication on their requests without implementing this in their own projects. Note: I also ran the tests (`swift test`) with the `docker.io/library/swift:6.0-focal` and `docker.io/library/swift:5.10.1-focal` to ensure linux compatibility. Signed-off-by: Agam Dua <[email protected]>
Migrate CI to use GitHub Actions. ### Motivation: To migrate to GitHub actions and centralised infrastructure. ### Modifications: Changes of note: * Adopt swift-format using rules from SwiftNIO. * Remove scripts and docker files which are no longer needed. * Disabled warnings-as-errors on Swift 6.0 CI pipelines for now. ### Result: Feature parity with old CI.
remove contributors script
remove unused Swift 6 language mode workflow
Enable MemberImportVisibility check on all targets. Use a standard string header and footer to bracket the new block for ease of updating in the future with scripts.
Update the release.yml file with the latest label changes
# Motivation The NIO 2.78 release introduced a bunch of new warnings. These warnings cause us a bunch of trouble, so we should fix them. # Modifications Mostly use a bunch of assumeIsolated() and syncOperations. # Result CI passes again. Note that Swift 6 has _many_ more warnings than this, but we expect more to come and we aren't using warnings-as-errors on that mode at the moment. We'll be cleaning that up soon.
This PR adds support for Android, mostly just by importing the Android module when needed.
### Motivation: In some cases we can crash because of a precondition failure when the write timeout fires and we aren't in the running state. This can happen for example if the connection is closed whilst the write timer is active. ### Modifications: * Remove the precondition and instead take no action if the timeout fires outside of the running state. Instead we take a new `Action`, `.noAction` when the timer fires. * Clear write timeouts upon request completion. When a request completes we have no use for the idle write timer, we clear the read timer and we should clear the write one too. ### Result: Fewer crashes. The supplied tests fails without these changes and passes with either of them.
CI use 6.1 nightlies now that Swift development is happening in the 6.1 branch
At the moment, `HTTPClient`'s entire API surface violates Structured Concurrency. Both the creation & shutdown of a HTTP client as well as making requests (#807) doesn't follow Structured Concurrency. Some of the problems are: 1. Upon return of methods, resources are still in active use in other threads/tasks 2. Cancellation doesn't always work This PR is baby steps towards a Structured Concurrency API, starting with start/shutdown of the HTTP client. Co-authored-by: Johannes Weiss <[email protected]>
Specifically Swift 5.10 _on Intel on Ubuntu Noble (24.04)_ has a crazy bug which leads to compilation failures in a `#if compiler(>=6.0)` block: swiftlang/swift#79285 . This workaround fixes the compilation by _changing the whitespace_. Thanks @gwynne for finding this workaround! --------- Co-authored-by: Johannes Weiss <[email protected]>
Motivation EmbeddedEventLoop is not thread-safe, which means that outside of very rare use-cases it's not safe to use it in Swift Concurrency. Modifications Replace invalid uses of EmbeddedEventLoop with NIOAsyncTestingEventLoop Result Better safety
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update the release.yml file with the latest label changes