connectrpc / connect-swift

The Swift implementation of Connect: Protobuf RPC that works.
https://connectrpc.com/docs/swift/getting-started
Apache License 2.0
93 stars 17 forks source link

Update SwiftLint to v0.55.1 #290

Closed rebello95 closed 1 month ago

rebello95 commented 1 month ago

Updates our version of SwiftLint, but opts out of one of the new rules because making the required changes:

--- a/Libraries/Connect/Internal/Interceptors/GRPCWebInterceptor.swift
+++ b/Libraries/Connect/Internal/Interceptors/GRPCWebInterceptor.swift
@@ -260,17 +260,11 @@ extension GRPCWebInterceptor: StreamInterceptor {
     }
 }

-private struct TrailersDecodingError: Error {}
-
 private extension Trailers {
     static func fromGRPCHeadersBlock(_ source: Data) throws -> Self {
-        guard let string = String(data: source, encoding: .utf8) else {
-            throw TrailersDecodingError()
-        }
-
         // Decode trailers based on gRPC Web spec:
         // https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md
-        return string
+        return String(decoding: source, as: UTF8.self)
             .split(separator: "\r\n")
             .reduce(into: Trailers()) { trailers, line in
                 guard let separatorIndex = line.firstIndex(of: ":") else {

Caused some tests to fail:

FAILED: gRPC-Web Unexpected Responses/HTTPVersion:1/TLS:false/trailers-in-body/unary/multiple-responses:
    actual error {code: 2 (unknown), message: "RPC response missing status"} does not match expected code 12 (unimplemented)

It looks like there was some discussion around this rule being potentially problematic here: https://github.com/realm/SwiftLint/issues/5263#issuecomment-2115182747

rebello95 commented 1 month ago

Thanks for the review @eseay! Honestly I noticed because of the failing test and then looked into it more. Surprising that there's an actual behavior change between the existing function and the suggested replacement...