Skip to content

Conversation

@itsmeichigo
Copy link
Contributor

Closes: #8511

Description

After attempting to fix #8511 with #8518, I could sometimes still be able to reproduce the issue that site address info not being loaded properly. I looked at Xcode console today and found an error:

⛔️ Cannot fetch WordPress site info: Error Domain=NSURLErrorDomain Code=-999 "cancelled" UserInfo={...}

It seems that the request was cancelled for some reason, and this issue doesn't happen consistently. It didn't seem to be an issue with DefaultStoresManager - but I checked Yosemite and found the part where site info is fetched suspicious. I switched to setting the whole Task to run from the main thread instead of using MainActor.run for the callback closure, which seems to stop the request from being cancelled.

I found an article where Paul Hudson shared an interesting point:

If your function is already running on the main actor, using await MainActor.run() will run your code immediately without waiting for the next run loop, but using Task as shown above (with @MainActor in) will wait for the next run loop.

I wonder if this has anything to do with the issue - I can't really find a proper explanation for this. My observation is that using MainActor.run, the fetch request fails in every 3 tries, while I can't seem to reproduce the issue with the other approach.

Testing instructions

  • Turn on the feature flag for applicationPasswordAuthenticationForSiteCredentialLogin and build the app.
  • Log out of the app or skip onboarding if needed.
  • On the prologue screen, select Enter your site address and enter the address of your self-hosted site.
  • Proceed to log in with your site credentials.
  • When the login succeed, you should be navigated to the home screen. Notice that the site name on the home screen and the menu tab are both correct.
  • Kill and relaunch the app as many time as you want. The site name should still be correct on both those screens.

Screenshots

N/A


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@itsmeichigo itsmeichigo added type: bug A confirmed bug. status: feature-flagged Behind a feature flag. Milestone is not strongly held. feature: REST API Authenticating requests using application password and using REST API instead of Jetpack tunnel. labels Jan 10, 2023
@itsmeichigo itsmeichigo added this to the 11.9 milestone Jan 10, 2023
@wpmobilebot
Copy link
Collaborator

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8607-415f029 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@itsmeichigo itsmeichigo modified the milestones: 11.9 ❄️, 12.0 Jan 16, 2023
@jaclync jaclync self-assigned this Jan 16, 2023
Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my understanding from the Swift concurrency talks last week, the function of the async call determines the thread it's run on - in this case, WordPressSiteStore.fetchSiteInfo is always run on the main thread (we have a main thread assertion for any Yosemite action), so theoretically the task is run on the main thread by default.

In any case, the changes look good to me 👍 The new version looks like the standard to me.

@itsmeichigo itsmeichigo merged commit 52f3aab into trunk Jan 16, 2023
@itsmeichigo itsmeichigo deleted the issue/8511-site-info-fetching branch January 16, 2023 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: REST API Authenticating requests using application password and using REST API instead of Jetpack tunnel. status: feature-flagged Behind a feature flag. Milestone is not strongly held. type: bug A confirmed bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[REST API] Invalid store address in My Store

4 participants