Moya: Upload progress incorrect due to wrong payload size

[matthiasotto]

Version 11.0.0-beta.2 (and 10 as well)

Using Task's.requestData(Data) leads to progress jumping straight to 1.0 due to size not being recognized properly. The same is not true just using straight up Alamofire.

Reported progress using Moya

🔢 preparing HTTP Task: .requestData with 2575959 bytes of data
(moya) 💚 progress: ProgressResponse(response: nil, progressObject: Optional(<NSProgress: 0x1d413ee60> : Parent: 0x0 / Fraction completed: 1.0000 / Completed: 11 of 11  ))
(moya) 💚 progress: ProgressResponse(response: Optional(Status Code: 200, Data Length: 11), progressObject: Optional(<NSProgress: 0x1d413ee60> : Parent: 0x0 / Fraction completed: 1.0000 / Completed: 11 of 11  ))
💚 Optional(200)

Reported progress using Alamofire

🔢 length = 2566021
(alamo) ❤️ progress: <NSProgress: 0x1d012e100> : Parent: 0x0 / Fraction completed: 0.0032 / Completed: 8192 of 2566021  
(alamo) ❤️ progress: <NSProgress: 0x1d012e100> : Parent: 0x0 / Fraction completed: 0.7119 / Completed: 1826816 of 2566021  
(alamo) ❤️ progress: <NSProgress: 0x1d012e100> : Parent: 0x0 / Fraction completed: 1.0000 / Completed: 2566021 of 2566021  
❤️ Optional(200)

Moya Service

enum ImageService {
    case upload(vehicleId: String, data: Data, id: String, token: String)
}

extension ImageService : TargetType {
    var baseURL: URL { return URL(string: "https://putsreq.com")! }

    var path: String { return "/Rvq2FIlE001dZLisBGxs" }

    var method: Moya.Method { return .post }

    var sampleData: Data { return Data() }

    var task: Task {
        switch self {

        case .upload(_, let data, _, _):
            print("🔢 preparing HTTP Task: .requestData with \(data.count) bytes of data")

            return .requestData(data)
        }
    }

    var validate: Bool { return false }

    var headers: [String: String]? {
        return ["Content-Type": "image/jpeg"]
    }
}

Moya Service invocation

    private func uploadWithMoyaTest(_ data: Data) {

        imageProvider.request(.upload(vehicleId: "1", data: data, id: "123", token: "456"), callbackQueue: DispatchQueue.main,
                              progress: { progress in print("(moya) 💚 progress: \(progress)") }
        ) { result in

            if case let .success(moyaResponse) = result {
                if let _ = try? moyaResponse.filterSuccessfulStatusCodes() {
                    print("💚 \(moyaResponse.response?.statusCode)")
                    return
                }
            }
            print("💚 Upload failed")
        }
    }

Alamo Service

class AlamoImageService {

    public static func upload(_ data: Data) throws -> UploadRequest {

        print("🔢 length = \(data.count)")

        let req = try URLRequest(url: "https://putsreq.com/B7LUB963aaAadyJ0XCwN".asURL(), method: HTTPMethod.post, headers: ["Content-Type":"image/jpeg"])

        return Alamofire.upload(data, with: req)
    }
}

Alamo invocation

    private func uploadWithAlamoTest(_ data: Data) {
        do {
            let uploadRequest = try AlamoImageService.upload(data)

            uploadRequest.uploadProgress { progress in
                print("(alamo) ❤️ progress: \(progress)")
            }

            uploadRequest.response { response in
                print("❤️ \(response.response?.statusCode)")
            }
        } catch {
            print("❤️ Upload failed")
        }
    }

[matthiasotto]

Side note: .uploadFile(URL) accurately reports size and progress.

[sunshinejr]

Hey @matthiasotto. First of all, thank you _so_ much for this detailed bug report. Means a lot.

I've copied your code into my Moya project, but I was getting the same (bad) results for both Moya and Alamofire. Would you be so kind to create a sample project with images that reproduce this problem? Then I would be able to investigate the problem more.

[stale[bot]]

This issue has been marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[SD10]

Mr. Stale Bot please don’t close this issue just yet

_{Sent with GitHawk}

[stale[bot]]

This issue has been marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale[bot]]

This issue has been auto-closed because there hasn't been any activity for at least 21 days. However, we really appreciate your contribution, so thank you for that! 🙏 Also, feel free to open a new issue if you still experience this problem 👍.

[sunshinejr]

I think we should still investigate the issue, so reopening this one. Unfortunately, I don't really have time to do it. Maybe some of the @Moya/contributors would help?

[zhongwuzw]

@matthiasotto , Based on your example demo, I think it's not comparable, because you use .requestData(data) when use Moya, it's a request operation, the progress indicates the download progress, but in Alamofire example, you use upload, the progress indicates the upload progress.

[sunshinejr]

Oh, that's a very good catch @zhongwuzw! Didn't see that one and I was looking at this issue for a few times.

[ffittschen]

Then I'd vote for a new case uploadData(Data) in Task.swift that maps to the Alamofire.upload API. In a current implementation of mine I also save data to a temporary file just to get the upload progress

[SD10]

Another request to add a new case to the Task enum 😢 I feel your pain on that one @ffittschen

[ffittschen]

Yes I also followed #1647. Maybe we need to rethink the Task? I don't know the original reasoning, but the documentation says a Task Represents an HTTP task. therefore it also feels wrong to me to add a new case just to upload data instead of files. Because from the HTTP perspective, the _task_ is the same.

[SeRG1k17]

@ffittschen I agree, we must revise the Task. In order that he could satisfy all requests.

[freak4pc]

I think task probably has to be some sort of Protocol by itself with some default static let "cases" so it could be easily extended if needed.

[SD10]

@freak4pc I don't think we can use a protocol here because Task has no specific implementation requirements. A lot of the context for these cases is provided by Moya internals as defaults.

@ffittschen There is #1556 discussing this. I partially agree with some the suggestions there

[stale[bot]]

This issue has been marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale[bot]]

This issue has been auto-closed because there hasn't been any activity for at least 21 days. However, we really appreciate your contribution, so thank you for that! 🙏 Also, feel free to open a new issue if you still experience this problem 👍.

[SD10]

I moved this discussion of this to #1655