From a312e73a108440e2bdb299d3ee4ec593ec841761 Mon Sep 17 00:00:00 2001 From: Lamberto Basti Date: Fri, 2 Feb 2024 16:44:43 +0100 Subject: [PATCH] 232 - Implement error handling for search, added log debugging for search caches. (#54) * Implement error handling for package search, better debugging for search caches. Added new functionality to handle search errors during package search, allowing users to retry the search. Included visual error representation in the UI with a retry link. The search can now also be retried programmatically if it fails. * Allow search error message to be collapsed. --- .../packagesearch/plugin/core/utils/Utils.kt | 5 +- .../model/packageslist/PackageListBuilder.kt | 44 ++++++++++- .../ui/model/packageslist/PackageListItem.kt | 20 ++++- .../packageslist/PackageListItemEvent.kt | 5 ++ .../packageslist/PackageListViewModel.kt | 68 +++++++++------- .../plugin/ui/model/packageslist/Search.kt | 77 +++++++++++++------ .../packages/PackageSearchPackageList.kt | 19 +++++ .../utils/PackageSearchApiPackageCache.kt | 11 ++- .../messages/packageSearchBundle.properties | 2 + 9 files changed, 188 insertions(+), 63 deletions(-) diff --git a/plugin/core/src/main/kotlin/com/jetbrains/packagesearch/plugin/core/utils/Utils.kt b/plugin/core/src/main/kotlin/com/jetbrains/packagesearch/plugin/core/utils/Utils.kt index c4a02670..c5a6ae92 100644 --- a/plugin/core/src/main/kotlin/com/jetbrains/packagesearch/plugin/core/utils/Utils.kt +++ b/plugin/core/src/main/kotlin/com/jetbrains/packagesearch/plugin/core/utils/Utils.kt @@ -36,6 +36,7 @@ import com.jetbrains.packagesearch.plugin.core.data.PackageSearchDeclaredPackage import com.jetbrains.packagesearch.plugin.core.services.PackageSearchProjectCachesService import java.nio.file.Path import kotlin.contracts.contract +import kotlin.coroutines.cancellation.CancellationException import kotlinx.coroutines.channels.ProducerScope import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow @@ -308,4 +309,6 @@ class ProjectDataImportListenerAdapter(private val project: Project) : ProjectDa override fun onFinalTasksFinished(projectPath: String?) { project.service().value = false } -} \ No newline at end of file +} + +fun Result.suspendSafe() = onFailure { if (it is CancellationException) throw it } diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListBuilder.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListBuilder.kt index 750ce198..62abd8f7 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListBuilder.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListBuilder.kt @@ -278,24 +278,60 @@ class PackageListBuilder( additionalContent = search.buildVariantsText() ) - is Search.Results.Base -> addFromSearchQueryBase( + is Search.Response.Base.Success -> addFromSearchQueryBase( headerId = headerId as PackageListItem.Header.Id.Remote.Base, search = search, module = modulesMap[headerId.moduleIdentity] as? PackageSearchModule.Base ?: return@forEach ) - is Search.Results.WithVariants -> addFromSearchQueryWithVariants( + is Search.Response.WithVariants.Success -> addFromSearchQueryWithVariants( headerId = headerId as PackageListItem.Header.Id.Remote.WithVariant, search = search, module = modulesMap[headerId.moduleIdentity] as? PackageSearchModule.WithVariants ?: return@forEach ) + + is Search.Response.Base.Error -> addSearchResultError(headerId = headerId) + + is Search.Response.WithVariants.Error -> addSearchResultError( + headerId = headerId, + attributes = search.attributes, + additionalContent = search.buildVariantsText() + ) } } } + private fun addSearchResultError( + headerId: PackageListItem.Header.Id.Remote, + attributes: List = emptyList(), + additionalContent: PackageListItem.Header.AdditionalContent.VariantsText? = null, + ) { + val headerState = when (headerCollapsedStates[headerId]) { + TargetState.OPEN -> PackageListItem.Header.State.OPEN + else -> PackageListItem.Header.State.CLOSED + } + addHeader( + title = PackageSearchBundle.message("packagesearch.ui.toolwindow.tab.packages.searchResults"), + id = headerId, + state = headerState, + attributes = attributes, + additionalContent = additionalContent, + ) + if (headerState == PackageListItem.Header.State.OPEN) { + items.add( + PackageListItem.SearchError( + id = PackageListItem.SearchError.Id( + headerId.moduleIdentity, + headerId + ) + ) + ) + } + } + private fun addFromSearchQueryWithVariants( headerId: PackageListItem.Header.Id.Remote.WithVariant, - search: Search.Results.WithVariants, + search: Search.Response.WithVariants.Success, module: PackageSearchModule.WithVariants, ) { val state = when (headerCollapsedStates[headerId]) { @@ -347,7 +383,7 @@ class PackageListBuilder( private fun addFromSearchQueryBase( headerId: PackageListItem.Header.Id.Remote.Base, - search: Search.Results.Base, + search: Search.Response.Base.Success, module: PackageSearchModule.Base, ) { val state = when (headerCollapsedStates[headerId]) { diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListItem.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListItem.kt index 177d8aab..6314206a 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListItem.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListItem.kt @@ -6,7 +6,6 @@ import kotlinx.serialization.Serializable sealed interface PackageListItem { - val title: String val id: Id @Serializable @@ -15,7 +14,7 @@ sealed interface PackageListItem { } data class Header( - override val title: String, + val title: String, override val id: Id, val state: State, val attributes: List = emptyList(), @@ -39,6 +38,7 @@ sealed interface PackageListItem { sealed interface Declared : Id { @Serializable data class Base(override val moduleIdentity: PackageSearchModule.Identity) : Declared + @Serializable data class WithVariant( override val moduleIdentity: PackageSearchModule.Identity, @@ -49,8 +49,10 @@ sealed interface PackageListItem { @Serializable sealed interface Remote : Id { + @Serializable data class Base(override val moduleIdentity: PackageSearchModule.Identity) : Remote + @Serializable data class WithVariant( override val moduleIdentity: PackageSearchModule.Identity, @@ -63,6 +65,8 @@ sealed interface PackageListItem { sealed interface Package : PackageListItem { + val title: String + @Serializable sealed interface Id : PackageListItem.Id { val packageId: String @@ -121,7 +125,7 @@ sealed interface PackageListItem { override val moduleIdentity: PackageSearchModule.Identity, override val packageId: String, val headerId: Header.Id.Remote.Base, - ) : Remote.Id + ) : Remote.Id } data class WithVariant( @@ -143,4 +147,14 @@ sealed interface PackageListItem { } } } + + data class SearchError(override val id: Id) : PackageListItem { + + @Serializable + data class Id( + override val moduleIdentity: PackageSearchModule.Identity, + val parentHeaderId: Header.Id, + ) : PackageListItem.Id + } + } \ No newline at end of file diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListItemEvent.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListItemEvent.kt index 92dced5e..758bebfd 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListItemEvent.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListItemEvent.kt @@ -115,4 +115,9 @@ sealed interface PackageListItemEvent { data class GoToSource(override val eventId: PackageListItem.Package.Declared.Id) : OnPackageAction } + + @Serializable + data class OnRetryPackageSearch( + override val eventId: PackageListItem.SearchError.Id, + ) : PackageListItemEvent } \ No newline at end of file diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListViewModel.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListViewModel.kt index bce9b410..5288d401 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListViewModel.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListViewModel.kt @@ -13,6 +13,7 @@ import com.jetbrains.packagesearch.plugin.core.data.PackageSearchDependencyManag import com.jetbrains.packagesearch.plugin.core.data.PackageSearchModule import com.jetbrains.packagesearch.plugin.core.data.PackageSearchModuleEditor import com.jetbrains.packagesearch.plugin.core.utils.IntelliJApplication +import com.jetbrains.packagesearch.plugin.core.utils.replayOn import com.jetbrains.packagesearch.plugin.fus.logGoToSource import com.jetbrains.packagesearch.plugin.fus.logHeaderAttributesClick import com.jetbrains.packagesearch.plugin.fus.logHeaderVariantsClick @@ -37,7 +38,6 @@ import com.jetbrains.packagesearch.plugin.utils.PackageSearchApplicationCachesSe import com.jetbrains.packagesearch.plugin.utils.PackageSearchProjectService import com.jetbrains.packagesearch.plugin.utils.logTODO import com.jetbrains.packagesearch.plugin.utils.logWarn -import com.jetbrains.packagesearch.plugin.utils.searchPackages import kotlin.time.Duration.Companion.milliseconds import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -138,6 +138,9 @@ class PackageListViewModel(private val project: Project) : Disposable { val searchQuery: String, ) + private val restartSearchChannel = Channel() + private val restartSearchFlow = restartSearchChannel.consumeAsFlow() + private val searchResultMapFlow: StateFlow> = combine( selectedModulesFlow, searchQueryStateFlow @@ -148,8 +151,9 @@ class PackageListViewModel(private val project: Project) : Disposable { else -> null } } + .replayOn(restartSearchFlow) .mapLatest { data -> - when (data) { + val map: Map = when (data) { null -> emptyMap() else -> { isLoadingChannel.send(true) @@ -160,9 +164,10 @@ class PackageListViewModel(private val project: Project) : Disposable { } } } + map } .onEach { isLoadingChannel.send(false) } - .modifiedBy(headerCollapsedStatesFlow) { current: Map, change -> + .modifiedBy(headerCollapsedStatesFlow) { current, change -> current.mapValues { (id, value) -> when { change[id] == OPEN && value is Search.Query -> value.execute() @@ -170,7 +175,7 @@ class PackageListViewModel(private val project: Project) : Disposable { } } } - .modifiedBy(selectedModulesFlow) { current: Map, change -> + .modifiedBy(selectedModulesFlow) { current, change -> val changeIdentities = change.map { it.identity } if (current.keys.any { it.moduleIdentity !in changeIdentities }) { emptyMap() @@ -215,23 +220,22 @@ class PackageListViewModel(private val project: Project) : Disposable { private suspend fun PackageSearchModule.Base.getSearchQuery( searchQuery: String, - ): Map { + ): Map { val headerId = PackageListItem.Header.Id.Remote.Base(identity) - val results = Search.Results.Base( - packages = IntelliJApplication.PackageSearchApplicationCachesService - .apiPackageCache - .searchPackages(buildSearchParameters { - this.searchQuery = searchQuery - packagesType = compatiblePackageTypes - }), - ) + val response = Search.Query.Base( + query = buildSearchParameters { + this.searchQuery = searchQuery + packagesType = compatiblePackageTypes + }, + apis = IntelliJApplication.PackageSearchApplicationCachesService.apiPackageCache, + ).execute() headerCollapsedStatesFlow.update { current -> when (headerId) { !in current -> current + (headerId to OPEN) else -> current } } - return mapOf(headerId to results) + return mapOf(headerId to response) } private suspend fun PackageSearchModule.WithVariants.getSearchQueries( @@ -251,19 +255,18 @@ class PackageListViewModel(private val project: Project) : Disposable { val primaryVariantName = variants.first { it.isPrimary }.name val attributes = variants.first().attributes.map { it.value } val additionalVariants = variants.map { it.name } - primaryVariantName - val search: Search = when (mainVariantName) { + val search = when (mainVariantName) { in variants.map { it.name } -> { - val results = Search.Results.WithVariants( - packages = IntelliJApplication.PackageSearchApplicationCachesService - .apiPackageCache - .searchPackages { - this.searchQuery = searchQuery - this.packagesType = packagesType - }, + val response = Search.Query.WithVariants( + query = buildSearchParameters { + this.searchQuery = searchQuery + this.packagesType = packagesType + }, + apis = IntelliJApplication.PackageSearchApplicationCachesService.apiPackageCache, attributes = attributes, primaryVariantName = primaryVariantName, additionalVariants = additionalVariants, - ) + ).execute() headerCollapsedStatesFlow.update { current -> when (headerId) { !in current -> current + (headerId to OPEN) @@ -271,7 +274,7 @@ class PackageListViewModel(private val project: Project) : Disposable { } } - results + response } else -> Search.Query.WithVariants( @@ -327,10 +330,15 @@ class PackageListViewModel(private val project: Project) : Disposable { is PackageListItemEvent.OnPackageAction.Update -> handle(event) is PackageListItemEvent.SetHeaderState -> handle(event) is PackageListItemEvent.UpdateAllPackages -> handle(event) + is PackageListItemEvent.OnRetryPackageSearch -> handle(event) } } } + private suspend fun handle(event: PackageListItemEvent.OnRetryPackageSearch) { + restartSearchChannel.send(Unit) + } + private fun handle(event: PackageListItemEvent.InfoPanelEvent.OnHeaderVariantsClick) { logTODO() logHeaderVariantsClick() @@ -353,7 +361,7 @@ class PackageListViewModel(private val project: Project) : Disposable { when (event.eventId) { is PackageListItem.Package.Remote.Base.Id -> { val headerId = PackageListItem.Header.Id.Remote.Base(event.eventId.moduleIdentity) - val search = searchResultMapFlow.value[headerId] as? Search.Results.Base + val search = searchResultMapFlow.value[headerId] as? Search.Response.Base.Success ?: return infoPanelViewModel.setPackage( module = event.eventId.getModule() as? PackageSearchModule.Base ?: return, @@ -367,8 +375,9 @@ class PackageListViewModel(private val project: Project) : Disposable { event.eventId.moduleIdentity, event.eventId.headerId.compatibleVariantNames ) - val search = searchResultMapFlow.value[headerId] as? Search.Results.WithVariants - ?: return + val search = + searchResultMapFlow.value[headerId] as? Search.Response.WithVariants.Success + ?: return infoPanelViewModel.setPackage( module = event.eventId.getModule() as? PackageSearchModule.WithVariants ?: return, apiPackage = search.packages.firstOrNull { it.id == event.eventId.packageId } ?: return, @@ -467,7 +476,7 @@ class PackageListViewModel(private val project: Project) : Disposable { val variant = module.variants[actionType.selectedVariantName] ?: return val search = searchResultMapFlow - .value[actionType.headerId] as? Search.Results.WithVariants + .value[actionType.headerId] as? Search.Response.WithVariants.Success ?: return val apiPackage = search.packages .firstOrNull { it.id == actionType.eventId.packageId } @@ -486,7 +495,8 @@ class PackageListViewModel(private val project: Project) : Disposable { val module = actionType.eventId .getModule() as? PackageSearchModule.Base ?: return - val search = searchResultMapFlow.value[actionType.headerId] as? Search.Results ?: return + val search = + searchResultMapFlow.value[actionType.headerId] as? Search.Response.Base.Success ?: return val apiPackage = search.packages .firstOrNull { it.id == actionType.eventId.packageId } ?: return diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/Search.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/Search.kt index be200c22..d3229211 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/Search.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/Search.kt @@ -1,5 +1,6 @@ package com.jetbrains.packagesearch.plugin.ui.model.packageslist +import com.jetbrains.packagesearch.plugin.core.utils.suspendSafe import org.jetbrains.packagesearch.api.v3.ApiPackage import org.jetbrains.packagesearch.api.v3.http.PackageSearchApi import org.jetbrains.packagesearch.api.v3.http.SearchPackagesRequest @@ -14,44 +15,74 @@ sealed interface Search { sealed interface Query : Search { - suspend fun execute(): Results + val query: SearchPackagesRequest + val apis: PackageSearchApi + + suspend fun execute(): Response data class Base( - val query: SearchPackagesRequest, - val apis: PackageSearchApi, + override val query: SearchPackagesRequest, + override val apis: PackageSearchApi, ) : Query { - override suspend fun execute(): Results.Base = - Results.Base(apis.searchPackages(query)) + override suspend fun execute(): Response.Base { + val searchResult = + kotlin.runCatching { apis.searchPackages(query) } + .suspendSafe() + .map { Response.Base.Success(it) } + val error = searchResult.exceptionOrNull() + return if (error != null) Response.Base.Error(error) else searchResult.getOrThrow() + } + } data class WithVariants( - val query: SearchPackagesRequest, - val apis: PackageSearchApi, + override val query: SearchPackagesRequest, + override val apis: PackageSearchApi, override val attributes: List, override val primaryVariantName: String, override val additionalVariants: List, ) : Query, Search.WithVariants { - override suspend fun execute(): Results = - Results.WithVariants( - packages = apis.searchPackages(query), - attributes = attributes, - primaryVariantName = primaryVariantName, - additionalVariants = additionalVariants - ) + override suspend fun execute(): Response.WithVariants { + val searchResult = + kotlin.runCatching { apis.searchPackages(query) } + .suspendSafe() + .map { Response.WithVariants.Success(it, attributes, primaryVariantName, additionalVariants) } + val error = searchResult.exceptionOrNull() + return when { + error != null -> + Response.WithVariants.Error(error, attributes, primaryVariantName, additionalVariants) + + else -> searchResult.getOrThrow() + } + } } } - sealed interface Results : Search { - val packages: List + sealed interface Response : Search { - data class Base(override val packages: List) : Results + sealed interface Base : Response { + + data class Success(val packages: List) : Base + data class Error(val error: Throwable) : Base + } + + sealed interface WithVariants : Response, Search.WithVariants { + + data class Success( + val packages: List, + override val attributes: List, + override val primaryVariantName: String, + override val additionalVariants: List, + ) : WithVariants + + data class Error( + val error: Throwable, + override val attributes: List, + override val primaryVariantName: String, + override val additionalVariants: List, + ) : WithVariants + } - data class WithVariants( - override val packages: List, - override val attributes: List, - override val primaryVariantName: String, - override val additionalVariants: List, - ) : Results, Search.WithVariants } } \ No newline at end of file diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/panels/packages/PackageSearchPackageList.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/panels/packages/PackageSearchPackageList.kt index 0d894f87..305bfb4f 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/panels/packages/PackageSearchPackageList.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/panels/packages/PackageSearchPackageList.kt @@ -104,11 +104,29 @@ fun PackageSearchPackageList( onPopupDismissRequest = { openPopupId = null } ) } + + is PackageListItem.SearchError -> item(key = item.id, contentType = item.contentType()) { + SearchErrorItem( + onLinkClick = { onPackageEvent(PackageListItemEvent.OnRetryPackageSearch(item.id)) } + ) + } } } } } +@Composable +fun SearchErrorItem(onLinkClick: () -> Unit) { + Column( + Modifier.fillMaxSize().padding(20.dp), + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally, + ) { + Text(message("packagesearch.ui.toolwindow.tab.packages.searchResults.error")) + Link(message("packagesearch.ui.toolwindow.tab.packages.searchResults.error.retry"), onClick = onLinkClick) + } +} + @Composable internal fun SelectableLazyItemScope.PackageListItem( modifier: Modifier = Modifier, @@ -405,6 +423,7 @@ private fun PackageListItem.contentType() = when (this) { is PackageListItem.Header -> "header" is PackageListItem.Package.Declared -> "declared.package" is PackageListItem.Package.Remote -> "remote.package" + is PackageListItem.SearchError -> "search.error" } @Composable diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/utils/PackageSearchApiPackageCache.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/utils/PackageSearchApiPackageCache.kt index cc0085f8..73f10db9 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/utils/PackageSearchApiPackageCache.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/utils/PackageSearchApiPackageCache.kt @@ -51,16 +51,21 @@ class PackageSearchApiPackageCache( override suspend fun searchPackages(request: SearchPackagesRequest): List { val sha = SHA256.digest(Json.encodeToString(request).toByteArray()).base64 + logDebug("${this::class}#searchPackages") { "Searching for packages | searchSha = $sha" } val cachedEntry = searchCache.find(NitriteFilters.Object.eq(ApiSearchEntry::searchHash, sha)) .singleOrNull() - if (cachedEntry != null) { - if (cachedEntry.lastUpdate + maxAge > Clock.System.now()) { + val isOffline = !isOnline() + val isCacheValid = cachedEntry.lastUpdate + maxAge > Clock.System.now() + if (isOffline || isCacheValid) { + logDebug("${this::class}#searchPackages") { + "Using cached search results because `isOffline = $isOffline || isCacheValid = $isCacheValid` | searchSha = $sha" + } return cachedEntry.packages } searchCache.remove(NitriteFilters.Object.eq(ApiSearchEntry::searchHash, sha)) } - + logDebug("${this::class}#searchPackages") { "Fetching search results from the server | searchSha = $sha" } return apiClient.searchPackages(request) .also { searchCache.insert(ApiSearchEntry(it, sha, request)) } } diff --git a/plugin/src/main/resources/messages/packageSearchBundle.properties b/plugin/src/main/resources/messages/packageSearchBundle.properties index 5491048d..e4fbc180 100644 --- a/plugin/src/main/resources/messages/packageSearchBundle.properties +++ b/plugin/src/main/resources/messages/packageSearchBundle.properties @@ -195,6 +195,8 @@ packagesearch.ui.toolwindow.packages.sort.by=Sort by: packagesearch.ui.toolwindow.tab.packages.installedPackages.addedIn=Added in {0} packagesearch.ui.toolwindow.tab.packages.searchResults=Search Results +packagesearch.ui.toolwindow.tab.packages.searchResults.error=An error occurred while searching for dependencies. +packagesearch.ui.toolwindow.tab.packages.searchResults.error.retry=Try again packagesearch.ui.toolwindow.tab.packages.selectedPackage=Selected dependency packagesearch.ui.toolwindow.tab.packages.title=Manage packagesearch.ui.toolwindow.tab.repositories.no.repositories.configured=There are no repositories configured in the current project