Bugfix FXIOS-10832 - Made pocket provider fetchStories to be async to fix withCheckedContinuation crash #23680
Conversation
…ix withCheckedContinuation crash
| func data(from urlRequest: URLRequest) async throws -> (Data, URLResponse) { | ||
| if let error = error { | ||
| throw error | ||
| } | ||
|
|
||
| return (data ?? Data(), response ?? URLResponse()) | ||
| } | ||
|
|
There was a problem hiding this comment.
This isn't called anywhere is it...? 🤔 Can be deleted?
There was a problem hiding this comment.
Yes & No
I know that wasn't the answer you were looking for but basically WallpaperURLSessionMock conforms to URLSessionProtocol, which has this additional method. Now we don't really need this for wallpaper but I added it for now to test if everything works fine. This is why I said "Yes" we need it for now but "No" if there is a better way to make it optional other than obj-c type protocol method then we can remove it. Going to tag Roux as he has been in this area more than me.
@adudenamedruby feel free to chime in here as well.
There was a problem hiding this comment.
Ahh ok, that makes sense then! I think I wasn't looking close enough to see it was in the unit tests. 🙂
| do { | ||
| return try await fetchStories(items: items) | ||
| } catch { | ||
| throw error |
There was a problem hiding this comment.
| do { | |
| return try await fetchStories(items: items) | |
| } catch { | |
| throw error | |
| return try await fetchStories(items: items) |
nit: I think I spotted one more! But I'm going to approve anyway. 👍
There was a problem hiding this comment.
You spoke too soon @ih-codes, I have more changes coming because tests are broken 🤣 (which I had a feeling anyways)
There was a problem hiding this comment.
Oh no!! 😂 Well you can just re-flag me for review if needed but I'm sure your tests will be great! 👌
There was a problem hiding this comment.
All done! Its not hard to fix these but had to find time. Please double check in case I missed something in my cleanup 🙈
There was a problem hiding this comment.
Looks good to me @nbhasin2 ! 👌
Client.app: Coverage: 31.97
Generated by 🚫 Danger Swift against f7e4cd4 |
📜 Tickets
Jira ticket
Github issue
💡 Description
Made pocket provider fetchStories to be async to fix withCheckedContinuation crash
📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120)