Skip to content

Commit 9b38f59

Browse files
committed
Contain unexpected throws in SiteXmlRpcUrlRecoverer
discoverAndVerifyXmlRpcUrl caught only DiscoveryException, so a RuntimeException from the discovery/verify path would escape the async, cancel the whole provisioning coroutine, and reach appScope (no SupervisorJob/handler). Catch CancellationException + generic Exception like the sibling SiteApiRestUrlRecoverer, so any failure degrades to the XML-RPC-disabled card and retries next run.
1 parent bab2b77 commit 9b38f59

2 files changed

Lines changed: 28 additions & 3 deletions

File tree

WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteXmlRpcUrlRecoverer.kt

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@ import org.wordpress.android.util.AppLog
1212
import javax.inject.Inject
1313
import javax.inject.Named
1414
import javax.inject.Singleton
15+
import kotlin.coroutines.cancellation.CancellationException
1516

1617
/**
1718
* Heals [SiteModel.xmlRpcUrl] for true self-hosted sites whose XML-RPC endpoint was never
1819
* discovered (or was previously gated). The REST counterpart is [SiteApiRestUrlRecoverer]; this
1920
* follows the same shape so callers never hold a mutated [SiteModel]:
2021
*
2122
* - [discoverAndVerifyXmlRpcUrl] discovers the endpoint and confirms it works with an authenticated
22-
* call, returning the verified URL (or `null` if discovery/verification fails).
23+
* call using the site's application-password credentials, returning the verified URL (or `null`
24+
* if discovery/verification fails).
2325
* - [persistXmlRpcUrl] writes only that one column to the DB row for `localId`.
2426
*/
2527
@Singleton
@@ -30,7 +32,7 @@ class SiteXmlRpcUrlRecoverer @Inject constructor(
3032
private val appLogWrapper: AppLogWrapper,
3133
@param:Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher,
3234
) {
33-
@Suppress("SwallowedException")
35+
@Suppress("SwallowedException", "TooGenericExceptionCaught")
3436
suspend fun discoverAndVerifyXmlRpcUrl(site: SiteModel): String? = withContext(bgDispatcher) {
3537
try {
3638
val endpoint = selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(site.url)
@@ -45,9 +47,21 @@ class SiteXmlRpcUrlRecoverer @Inject constructor(
4547
} else {
4648
endpoint
4749
}
50+
} catch (e: CancellationException) {
51+
throw e
4852
} catch (e: SelfHostedEndpointFinder.DiscoveryException) {
53+
// Expected when the site has no reachable XML-RPC endpoint — surfaces as the
54+
// XML-RPC-disabled card (xmlRpcUrl stays empty) and retries on the next run.
4955
appLogWrapper.w(AppLog.T.API, "XML-RPC discovery failed for ${site.url}")
5056
null
57+
} catch (e: Exception) {
58+
// Best-effort recovery must never let an unexpected throw escape and cancel the
59+
// provisioning pipeline (mirrors SiteApiRestUrlRecoverer). Same null -> disabled-card surface.
60+
appLogWrapper.e(
61+
AppLog.T.API,
62+
"XML-RPC discovery threw for ${site.url}: ${e::class.simpleName}: ${e.message}"
63+
)
64+
null
5165
}
5266
}
5367

WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteXmlRpcUrlRecovererTest.kt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ class SiteXmlRpcUrlRecovererTest : BaseUnitTest() {
5656
@Test
5757
fun `given discovery and authenticated verify succeed, then returns the endpoint`() = test {
5858
whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(site.url)).thenReturn(ENDPOINT)
59-
whenever(siteXMLRPCClient.fetchSites(eq(ENDPOINT), any(), any()))
59+
// The site's stored credentials are forwarded to the authenticated verify call.
60+
whenever(siteXMLRPCClient.fetchSites(eq(ENDPOINT), eq("user"), eq("pass")))
6061
.thenReturn(SitesModel(listOf(SiteModel())))
6162

6263
assertThat(recoverer.discoverAndVerifyXmlRpcUrl(site)).isEqualTo(ENDPOINT)
@@ -70,6 +71,16 @@ class SiteXmlRpcUrlRecovererTest : BaseUnitTest() {
7071
assertThat(recoverer.discoverAndVerifyXmlRpcUrl(site)).isNull()
7172
}
7273

74+
@Test
75+
fun `given discovery throws an unexpected exception, then returns null`() = test {
76+
// A non-DiscoveryException (e.g. a RuntimeException from the network/parse path) must be
77+
// contained, not propagated — otherwise it cancels the whole provisioning pipeline.
78+
whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(site.url))
79+
.thenThrow(RuntimeException("unexpected"))
80+
81+
assertThat(recoverer.discoverAndVerifyXmlRpcUrl(site)).isNull()
82+
}
83+
7384
@Test
7485
fun `given the authenticated verify errors, then returns null`() = test {
7586
whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(site.url)).thenReturn(ENDPOINT)

0 commit comments

Comments
 (0)