Moya version: 11.0.2, but also tested with Moya 12
I am using Moya with RxSwift, using the requestWithProgress function to get the response and progress on uploading a file.
The progress part is working perfectly, however, I found that when I used the response, my other Rx operations on the provided Observable weren't working correctly. When I debugged what was going on, I noticed that the Observable I received from doing:
requestWithProgress(...).filterCompleted()
was only firing an onCompleted and then dispose. There was no onNext that contained the actual response.
To get this working instead of using the provided filterCompleted I just had to do it myself with:
requestWithProgress(...)
.map { $0.response }
.filter { $0 != nil }
.map { $0! }
In short, this is pulling out the response field, filtering for when it isn't nil and then unwrapping the optional.
When I use this instead, everything works as expected. I think the filterCompleted implementation is firing no onNext events. The type Observable<Response> and the comment on the function suggest that it should be providing an Observable that gives you the response and then an onCompleted event, so I think there is a bug there.
For now my code is working with what I put above, so its not blocking me from using this, but just raising this issue, as I think there is a bug that may be affecting others.
IIRC we've had issues with requestWithProgress before -- last time there were no .next events with progress. This sounds like it could be a real bug in Moya, I'm looking to find the issue that described the old bug
Actually, as I read closer, it may not be as deep-seated a bug as I imagined. Here's the implementation of filterCompleted: https://github.com/Moya/Moya/blob/9a4b2901492898acaceb232dc6481ecc85dfff50/Sources/RxMoya/Observable%2BResponse.swift#L65
Do you see any issues with it? If you edit it to work, I'd be more than happy to accept a PR from you
There's some history behind the implementation in the PR that stemmed from #1094, I'd recommend taking a look there to understand how filterCompleted / completed / requestWithProgress evolved
Thanks, will have a look when I get a chance.
I'm just using my own version right now rather than using filterCompleted,
so at least have an alternative for the time being.
Appreciate the links!
On Tue, 27 Nov 2018, 23:28 Andrew Breckenridge <[email protected]
wrote:
There's some history behind the implementation in the PR that stemmed from
1094 https://github.com/Moya/Moya/issues/1094, I'd recommend taking a
look there to understand how filterCompleted / completed /
requestWithProgress evolved—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/Moya/Moya/issues/1771#issuecomment-442258228, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AKy1k-0D3e10TD_SRQhiuR2N5F6eng4Fks5uzcqggaJpZM4Yo-nK
.
This issue has been marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
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 👍.
I've also run into this issue today. I have logged the progressObject and found this :
Fraction completed: 0.0000 / Completed: 243 of -1
It seems that the progress's totalUnitCount remains -1 while completedUnitCount increases. Which then leads to a fractionCompleted being always 0.
progress property of ProgressResponse is never 1 (based on progressObject's fractionCompleted)completed property is never true (based on progress value)filterCompleted() never succeeds (based on completed)The issue surely comes from the progressObject totalUnitCount being -1.
This may happens, based on this SO thread, when the response size (Content-Length header) is not known : https://stackoverflow.com/questions/23585432/totalbytesexpectedtowrite-is-1-in-nsurlsessiondownloadtask
That is super interesting, thanks for digging @ryancrunchi. How do you think we should handle the issue?
If we don't know what the total payload size is, I can't think of any better value than 0 to call with for progress. Would you agree? We could probably use some other piece of data from Alamofire to override the completed property
Yes, I agree, progress should remain 0.
But the filterCompleted should rely on an other condition than the fractionCompleted value.
yeah, that makes sense. If anyone would like to take a shot at fixing the behavior, I'd be more than happy to review a PR
I don't know what the state of this issue now, but I have tested that when a download is not complete, progressResponse.response is nil. Once the download is complete, progressResponse.response is not nil.
Maybe we could rely on this to filter the completed state of the download.
@lordcodes @ryancrunchi @AndrewSB FYI I've created a PR with a fix (#1815).
Closing it as the PR with the fix is merged. This should be released in Moya 13 in the near future! You can try it out using develop branch, but please don't use the code from develop on production yet. Thanks!