From f5df3808f2b56ae23aa46f521344cebcec70b82d Mon Sep 17 00:00:00 2001 From: "Chris (SPG) McGee" Date: Wed, 7 Aug 2024 13:21:58 -0400 Subject: [PATCH 1/7] Provide documentation and context information for NIOTooManyBytesError The NIOTooManyBytesError doesn't have any documentation for someone that encounters this error. Provide documentation that explains the situation when the error occurs, which is the upTo limit of an AsyncSequence is exceeded. Describe one potential action, which is to increase this limit. The error doesn't give the end user any sense of magnitude of the byte limit that has been exceeded. Provide the maxBytes in the error so that the user can gauge whether the upTo limit is already reasonable and the payload size is excessive, or the limit needs to be increased to suit more situations. --- Sources/NIOCore/AsyncAwaitSupport.swift | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/Sources/NIOCore/AsyncAwaitSupport.swift b/Sources/NIOCore/AsyncAwaitSupport.swift index abdc20c2d6..ef7b0ee473 100644 --- a/Sources/NIOCore/AsyncAwaitSupport.swift +++ b/Sources/NIOCore/AsyncAwaitSupport.swift @@ -240,8 +240,21 @@ extension ChannelPipeline { } } +/// An error that is thrown when the number of bytes in an AsyncSequence exceeds the limit. +/// +/// When collecting the bytes from an AsyncSequence, there is a limit up to where the content +/// exceeds a certain threshold beyond which the content isn't matching the expected maximum size. +/// that will be processed. This error is generally thrown when it is discovered that there are +/// more bytes in a sequence than what was specified as the maximum. It could be that this upTo +/// limit should be increased, or that the sequence has unexpected content in it. +/// +/// - maxBytes: the maximum number of bytes to be collected public struct NIOTooManyBytesError: Error, Hashable { - public init() {} + let maxBytes: Int + + public init(maxBytes: Int) { + self.maxBytes = maxBytes + } } @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) @@ -262,7 +275,7 @@ extension AsyncSequence where Element: RandomAccessCollection, Element.Element = for try await fragment in self { bytesRead += fragment.count guard bytesRead <= maxBytes else { - throw NIOTooManyBytesError() + throw NIOTooManyBytesError(maxBytes: maxBytes) } accumulationBuffer.writeBytes(fragment) } @@ -305,7 +318,7 @@ extension AsyncSequence where Element == ByteBuffer { for try await fragment in self { bytesRead += fragment.readableBytes guard bytesRead <= maxBytes else { - throw NIOTooManyBytesError() + throw NIOTooManyBytesError(maxBytes: maxBytes) } accumulationBuffer.writeImmutableBuffer(fragment) } @@ -328,7 +341,7 @@ extension AsyncSequence where Element == ByteBuffer { return ByteBuffer() } guard head.readableBytes <= maxBytes else { - throw NIOTooManyBytesError() + throw NIOTooManyBytesError(maxBytes: maxBytes) } let tail = AsyncSequenceFromIterator(iterator) From 48c4858600a323801039b1b2046538d4e39b2bfa Mon Sep 17 00:00:00 2001 From: "Chris (SPG) McGee" Date: Wed, 21 Aug 2024 16:10:45 -0400 Subject: [PATCH 2/7] Maintain API compatibility --- Sources/NIOCore/AsyncAwaitSupport.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/NIOCore/AsyncAwaitSupport.swift b/Sources/NIOCore/AsyncAwaitSupport.swift index ef7b0ee473..45c47461f1 100644 --- a/Sources/NIOCore/AsyncAwaitSupport.swift +++ b/Sources/NIOCore/AsyncAwaitSupport.swift @@ -248,9 +248,13 @@ extension ChannelPipeline { /// more bytes in a sequence than what was specified as the maximum. It could be that this upTo /// limit should be increased, or that the sequence has unexpected content in it. /// -/// - maxBytes: the maximum number of bytes to be collected +/// - maxBytes: (optional) current limit on the maximum number of bytes in the sequence public struct NIOTooManyBytesError: Error, Hashable { - let maxBytes: Int + let maxBytes: Int? + + public init() { + self.maxBytes = nil + } public init(maxBytes: Int) { self.maxBytes = maxBytes From 3c372bf182b05cdd35f4184baffe0cb2e2ad846c Mon Sep 17 00:00:00 2001 From: "Chris (SPG) McGee" Date: Thu, 22 Aug 2024 11:44:41 -0400 Subject: [PATCH 3/7] Deprecate the original constructor, make the maxBytes public mutable, and test maxBytes is set as expected --- Sources/NIOCore/AsyncAwaitSupport.swift | 8 +++++--- Tests/NIOCoreTests/AsyncSequenceTests.swift | 21 +++++++++++++++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/Sources/NIOCore/AsyncAwaitSupport.swift b/Sources/NIOCore/AsyncAwaitSupport.swift index 45c47461f1..20324deac7 100644 --- a/Sources/NIOCore/AsyncAwaitSupport.swift +++ b/Sources/NIOCore/AsyncAwaitSupport.swift @@ -12,6 +12,8 @@ // //===----------------------------------------------------------------------===// +import Foundation + extension EventLoopFuture { /// Get the value/error from an `EventLoopFuture` in an `async` context. /// @@ -247,11 +249,11 @@ extension ChannelPipeline { /// that will be processed. This error is generally thrown when it is discovered that there are /// more bytes in a sequence than what was specified as the maximum. It could be that this upTo /// limit should be increased, or that the sequence has unexpected content in it. -/// -/// - maxBytes: (optional) current limit on the maximum number of bytes in the sequence public struct NIOTooManyBytesError: Error, Hashable { - let maxBytes: Int? + /// Current limit on the maximum number of bytes in the sequence + public var maxBytes: Int? + @available(*, deprecated, message: "Construct the NIOTooManyBytesError with the maxBytes limit that triggered this error") public init() { self.maxBytes = nil } diff --git a/Tests/NIOCoreTests/AsyncSequenceTests.swift b/Tests/NIOCoreTests/AsyncSequenceTests.swift index 069519b40f..d8894140d0 100644 --- a/Tests/NIOCoreTests/AsyncSequenceTests.swift +++ b/Tests/NIOCoreTests/AsyncSequenceTests.swift @@ -126,10 +126,11 @@ final class AsyncSequenceCollectTests: XCTestCase { } // test for the generic version + let maxBytes = max(expectedBytes.count - 1, 0) await XCTAssertThrowsError( try await testCase.buffers .asAsyncSequence() - .collect(upTo: max(expectedBytes.count - 1, 0), using: .init()), + .collect(upTo: maxBytes, using: .init()), file: testCase.file, line: testCase.line ) { error in @@ -138,6 +139,14 @@ final class AsyncSequenceCollectTests: XCTestCase { file: testCase.file, line: testCase.line ) + if let tooManyBytesErr = error as? NIOTooManyBytesError { + XCTAssertEqual( + maxBytes, + tooManyBytesErr.maxBytes, + file: testCase.file, + line: testCase.line + ) + } } // test for the `ByteBuffer` optimised version @@ -145,7 +154,7 @@ final class AsyncSequenceCollectTests: XCTestCase { try await testCase.buffers .map(ByteBuffer.init(bytes:)) .asAsyncSequence() - .collect(upTo: max(expectedBytes.count - 1, 0)), + .collect(upTo: maxBytes), file: testCase.file, line: testCase.line ) { error in @@ -154,6 +163,14 @@ final class AsyncSequenceCollectTests: XCTestCase { file: testCase.file, line: testCase.line ) + if let tooManyBytesErr = error as? NIOTooManyBytesError { + // Sometimes the max bytes is subtracted from the header size + XCTAssertTrue( + tooManyBytesErr.maxBytes != nil && tooManyBytesErr.maxBytes! <= maxBytes, + file: testCase.file, + line: testCase.line + ) + } } } } From 3623af263db7788fae63f99dcc7c9f8bb149e799 Mon Sep 17 00:00:00 2001 From: "Chris (SPG) McGee" Date: Thu, 22 Aug 2024 12:13:05 -0400 Subject: [PATCH 4/7] Remove foundation import --- Sources/NIOCore/AsyncAwaitSupport.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Sources/NIOCore/AsyncAwaitSupport.swift b/Sources/NIOCore/AsyncAwaitSupport.swift index 20324deac7..a4447628f4 100644 --- a/Sources/NIOCore/AsyncAwaitSupport.swift +++ b/Sources/NIOCore/AsyncAwaitSupport.swift @@ -12,8 +12,6 @@ // //===----------------------------------------------------------------------===// -import Foundation - extension EventLoopFuture { /// Get the value/error from an `EventLoopFuture` in an `async` context. /// From 60f2764a5c51faee41dea07684b9ba1e6b825f55 Mon Sep 17 00:00:00 2001 From: "Chris (SPG) McGee" Date: Thu, 22 Aug 2024 13:08:17 -0400 Subject: [PATCH 5/7] Tighten up the description of the error --- Sources/NIOCore/AsyncAwaitSupport.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/NIOCore/AsyncAwaitSupport.swift b/Sources/NIOCore/AsyncAwaitSupport.swift index a4447628f4..955c4b9bd6 100644 --- a/Sources/NIOCore/AsyncAwaitSupport.swift +++ b/Sources/NIOCore/AsyncAwaitSupport.swift @@ -243,8 +243,8 @@ extension ChannelPipeline { /// An error that is thrown when the number of bytes in an AsyncSequence exceeds the limit. /// /// When collecting the bytes from an AsyncSequence, there is a limit up to where the content -/// exceeds a certain threshold beyond which the content isn't matching the expected maximum size. -/// that will be processed. This error is generally thrown when it is discovered that there are +/// exceeds a certain threshold beyond which the content isn't matching an expected reasonable +/// size to be processed. This error is generally thrown when it is discovered that there are more /// more bytes in a sequence than what was specified as the maximum. It could be that this upTo /// limit should be increased, or that the sequence has unexpected content in it. public struct NIOTooManyBytesError: Error, Hashable { From f065562f6f26bb6b2feaaa655e138c1d2b9060f7 Mon Sep 17 00:00:00 2001 From: "Chris (SPG) McGee" Date: Fri, 23 Aug 2024 11:47:42 -0400 Subject: [PATCH 6/7] Code review feedback --- Sources/NIOCore/AsyncAwaitSupport.swift | 16 ++++++++++- Tests/NIOCoreTests/AsyncSequenceTests.swift | 30 +++++++++++++++------ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/Sources/NIOCore/AsyncAwaitSupport.swift b/Sources/NIOCore/AsyncAwaitSupport.swift index 955c4b9bd6..19359b4b7e 100644 --- a/Sources/NIOCore/AsyncAwaitSupport.swift +++ b/Sources/NIOCore/AsyncAwaitSupport.swift @@ -247,7 +247,7 @@ extension ChannelPipeline { /// size to be processed. This error is generally thrown when it is discovered that there are more /// more bytes in a sequence than what was specified as the maximum. It could be that this upTo /// limit should be increased, or that the sequence has unexpected content in it. -public struct NIOTooManyBytesError: Error, Hashable { +public struct NIOTooManyBytesError: Error { /// Current limit on the maximum number of bytes in the sequence public var maxBytes: Int? @@ -261,6 +261,20 @@ public struct NIOTooManyBytesError: Error, Hashable { } } +extension NIOTooManyBytesError: Equatable { + public static func == (lhs: NIOTooManyBytesError, rhs: NIOTooManyBytesError) -> Bool { + // Equality of the maxBytes isn't of consequence + return true + } +} + +extension NIOTooManyBytesError: Hashable { + public func hash(into hasher: inout Hasher) { + // All errors of this type hash to the same value since maxBytes isn't of consequence + hasher.combine(7) + } +} + @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) extension AsyncSequence where Element: RandomAccessCollection, Element.Element == UInt8 { /// Accumulates an `AsyncSequence` of `RandomAccessCollection`s into a single `accumulationBuffer`. diff --git a/Tests/NIOCoreTests/AsyncSequenceTests.swift b/Tests/NIOCoreTests/AsyncSequenceTests.swift index d8894140d0..7168605868 100644 --- a/Tests/NIOCoreTests/AsyncSequenceTests.swift +++ b/Tests/NIOCoreTests/AsyncSequenceTests.swift @@ -139,14 +139,21 @@ final class AsyncSequenceCollectTests: XCTestCase { file: testCase.file, line: testCase.line ) - if let tooManyBytesErr = error as? NIOTooManyBytesError { - XCTAssertEqual( - maxBytes, - tooManyBytesErr.maxBytes, + guard let tooManyBytesErr = error as? NIOTooManyBytesError else { + XCTFail( + "Error was not an NIOTooManyBytesError", file: testCase.file, line: testCase.line ) + return } + + XCTAssertEqual( + maxBytes, + tooManyBytesErr.maxBytes, + file: testCase.file, + line: testCase.line + ) } // test for the `ByteBuffer` optimised version @@ -163,14 +170,21 @@ final class AsyncSequenceCollectTests: XCTestCase { file: testCase.file, line: testCase.line ) - if let tooManyBytesErr = error as? NIOTooManyBytesError { - // Sometimes the max bytes is subtracted from the header size - XCTAssertTrue( - tooManyBytesErr.maxBytes != nil && tooManyBytesErr.maxBytes! <= maxBytes, + guard let tooManyBytesErr = error as? NIOTooManyBytesError else { + XCTFail( + "Error was not an NIOTooManyBytesError", file: testCase.file, line: testCase.line ) + return } + + // Sometimes the max bytes is subtracted from the header size + XCTAssertTrue( + tooManyBytesErr.maxBytes != nil && tooManyBytesErr.maxBytes! <= maxBytes, + file: testCase.file, + line: testCase.line + ) } } } From 6a4b8afac643f2344235adaa9c90b00950a04bfa Mon Sep 17 00:00:00 2001 From: "Chris (SPG) McGee" Date: Tue, 27 Aug 2024 09:55:11 -0400 Subject: [PATCH 7/7] Fix formatting errors --- Sources/NIOCore/AsyncAwaitSupport.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/NIOCore/AsyncAwaitSupport.swift b/Sources/NIOCore/AsyncAwaitSupport.swift index 19359b4b7e..05e8061716 100644 --- a/Sources/NIOCore/AsyncAwaitSupport.swift +++ b/Sources/NIOCore/AsyncAwaitSupport.swift @@ -251,7 +251,11 @@ public struct NIOTooManyBytesError: Error { /// Current limit on the maximum number of bytes in the sequence public var maxBytes: Int? - @available(*, deprecated, message: "Construct the NIOTooManyBytesError with the maxBytes limit that triggered this error") + @available( + *, + deprecated, + message: "Construct the NIOTooManyBytesError with the maxBytes limit that triggered this error" + ) public init() { self.maxBytes = nil } @@ -264,7 +268,7 @@ public struct NIOTooManyBytesError: Error { extension NIOTooManyBytesError: Equatable { public static func == (lhs: NIOTooManyBytesError, rhs: NIOTooManyBytesError) -> Bool { // Equality of the maxBytes isn't of consequence - return true + true } }