Skip to content

Commit

Permalink
Merge pull request #612 from snyk/fix/shutdown-fixes-and-more
Browse files Browse the repository at this point in the history
fix: shutdown exception logging, and auto-scan on startup
  • Loading branch information
bastiandoetsch authored Sep 13, 2024
2 parents c26cf14 + 451ce4c commit a790ab7
Show file tree
Hide file tree
Showing 23 changed files with 116 additions and 62 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
- add flashes for auto-fixable Open Source Issues
- show code vision for Open Source also, when Snyk Code is still analysing
- clean-up old open source scan functionality
- don't print out exceptions during shutdown of the app/plugin
- if the language server listener is shut down, set initialized to false
- log error stream of language server to idea.log
- show error / warn messages if the project is null (e.g. for offline handling)

## [2.9.1]
### Fixed
Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/io/snyk/plugin/SnykPostStartupActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class SnykPostStartupActivity : ProjectActivity {
}

if (!settings.token.isNullOrBlank() && settings.scanOnSave) {
getSnykTaskQueueService(project)?.scan(true)
getSnykTaskQueueService(project)?.scan()
}

ExtensionPointsUtil.controllerManager.extensionList.forEach {
Expand Down
6 changes: 4 additions & 2 deletions src/main/kotlin/io/snyk/plugin/SnykProjectManagerListener.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ class SnykProjectManagerListener : ProjectManagerListener {
ls.updateWorkspaceFolders(emptySet(), ls.getWorkspaceFolders(project))
}
}.get(TIMEOUT, TimeUnit.SECONDS)
} catch (ignored: RuntimeException) {
logger<SnykProjectManagerListener>().info("Project closing clean up took too long", ignored)
} catch (ignored: Exception) {
val logger = logger<SnykProjectManagerListener>()
logger.warn("Project closing clean up took longer than $TIMEOUT seconds")
logger.debug(ignored)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class SnykControllerImpl(val project: Project) : SnykController {
* scan enqueues a scan of the project for vulnerabilities.
*/
override fun scan() {
getSnykTaskQueueService(project)?.scan(false)
getSnykTaskQueueService(project)?.scan()
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class SnykTaskQueueService(val project: Project) {
}
}

fun scan(isStartup: Boolean) {
fun scan() {
taskQueue.run(object : Task.Backgroundable(project, "Snyk: initializing...", true) {
override fun run(indicator: ProgressIndicator) {
if (!confirmScanningAndSetWorkspaceTrustedStateIfNeeded(project)) return
Expand All @@ -101,9 +101,7 @@ class SnykTaskQueueService(val project: Project) {
waitUntilCliDownloadedIfNeeded()
indicator.checkCanceled()

if (!isStartup) {
LanguageServerWrapper.getInstance().sendScanCommand(project)
}
LanguageServerWrapper.getInstance().sendScanCommand(project)

if (settings.iacScanEnabled) {
if (!isSnykIaCLSEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class SnykCliDownloaderService {
try {
if (languageServerWrapper.isInitialized) {
try {
languageServerWrapper.shutdown().get(2, TimeUnit.SECONDS)
languageServerWrapper.shutdown()
} catch (e: RuntimeException) {
logger<SnykCliDownloaderService>()
.warn("Language server shutdown for download took too long, couldn't shutdown", e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ object SnykBalloonNotificationHelper {
showNotification(message, project, NotificationType.ERROR, *actions)
}

fun showInfo(message: String, project: Project, vararg actions: AnAction) =
fun showInfo(message: String, project: Project?, vararg actions: AnAction) =
showNotification(message, project, NotificationType.INFORMATION, *actions)

fun showWarn(message: String, project: Project, vararg actions: AnAction) =
fun showWarn(message: String, project: Project?, vararg actions: AnAction) =
showNotification(message, project, NotificationType.WARNING, *actions)

private fun showNotification(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import io.snyk.plugin.pluginSettings
class SnykRunScanAction : AnAction(AllIcons.Actions.Execute), DumbAware {

override fun actionPerformed(actionEvent: AnActionEvent) {
getSnykTaskQueueService(actionEvent.project!!)?.scan(false)
getSnykTaskQueueService(actionEvent.project!!)?.scan()
}

override fun update(actionEvent: AnActionEvent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class IssueViewOptionsPanel(
.actionListener{ _, it ->
if (canBeChanged(it, it.isSelected)) {
currentOpenIssuesEnabled = it.isSelected
getSnykTaskQueueService(project)?.scan(false)
getSnykTaskQueueService(project)?.scan()
}
}
// bindSelected is needed to trigger apply() on the settings dialog that this panel is rendered in
Expand All @@ -44,7 +44,7 @@ class IssueViewOptionsPanel(
.actionListener{ _, it ->
if (canBeChanged(it, it.isSelected)) {
currentIgnoredIssuesEnabled = it.isSelected
getSnykTaskQueueService(project)?.scan(false)
getSnykTaskQueueService(project)?.scan()
}
}
.bindSelected(settings::ignoredIssuesEnabled)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ class SnykPluginDisposable : Disposable, AppLifecycleListener {

override fun appClosing() {
try {
LanguageServerWrapper.getInstance().shutdown().get(2, TimeUnit.SECONDS)
LanguageServerWrapper.getInstance().shutdown()
} catch (ignored: Exception) {
// do nothing
}
}

override fun appWillBeClosed(isRestart: Boolean) {
try {
LanguageServerWrapper.getInstance().shutdown().get(2, TimeUnit.SECONDS)
LanguageServerWrapper.getInstance().shutdown()
} catch (ignored: Exception) {
// do nothing
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ class SnykToolWindowPanel(
}

private fun triggerScan() {
getSnykTaskQueueService(project)?.scan(false)
getSnykTaskQueueService(project)?.scan()
}

fun displayAuthPanel() {
Expand Down
57 changes: 40 additions & 17 deletions src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import io.snyk.plugin.getSnykTaskQueueService
import io.snyk.plugin.getWaitForResultsTimeout
import io.snyk.plugin.isSnykIaCLSEnabled
import io.snyk.plugin.pluginSettings
import io.snyk.plugin.services.SnykApplicationSettingsStateService
import io.snyk.plugin.toLanguageServerURL
import io.snyk.plugin.ui.toolwindow.SnykPluginDisposable
import kotlinx.coroutines.DelicateCoroutinesApi
Expand All @@ -42,6 +41,7 @@ import org.eclipse.lsp4j.WorkspaceEditCapabilities
import org.eclipse.lsp4j.WorkspaceFolder
import org.eclipse.lsp4j.WorkspaceFoldersChangeEvent
import org.eclipse.lsp4j.jsonrpc.Launcher
import org.eclipse.lsp4j.jsonrpc.RemoteEndpoint
import org.eclipse.lsp4j.launch.LSPLauncher
import org.eclipse.lsp4j.services.LanguageServer
import org.jetbrains.concurrency.runAsync
Expand All @@ -62,10 +62,10 @@ import snyk.trust.WorkspaceTrustService
import snyk.trust.confirmScanningAndSetWorkspaceTrustedStateIfNeeded
import java.util.concurrent.ExecutorService
import java.util.concurrent.Executors
import java.util.concurrent.Future
import java.util.concurrent.TimeUnit
import java.util.concurrent.TimeoutException
import java.util.concurrent.locks.ReentrantLock
import java.util.logging.Logger.getLogger
import kotlin.io.path.exists

private const val INITIALIZATION_TIMEOUT = 20L
Expand Down Expand Up @@ -134,39 +134,62 @@ class LanguageServerWrapper(
EnvironmentHelper.updateEnvironment(processBuilder.environment(), pluginSettings().token ?: "")

process = processBuilder.start()
launcher = LSPLauncher.createClientLauncher(languageClient, process.inputStream, process.outputStream)
languageServer = launcher.remoteProxy

GlobalScope.launch {
if (!disposed) {
try {
process.errorStream.bufferedReader().forEachLine { println(it) }
process.errorStream.bufferedReader().forEachLine { logger.debug(it) }
} catch (ignored: Exception) {
// ignore
}
}
}

launcher.startListening()
sendInitializeMessage()
isInitialized = true
launcher = LSPLauncher.createClientLauncher(languageClient, process.inputStream, process.outputStream)
languageServer = launcher.remoteProxy

val listenerFuture = launcher.startListening()

runAsync {
listenerFuture.get()
isInitialized = false
}

if (!listenerFuture.isDone) {
sendInitializeMessage()
isInitialized = true
} else {
logger.warn("Language Server initialization did not succeed")
}
} catch (e: Exception) {
logger.warn(e)
process.destroy()
isInitialized = false
}
}

fun shutdown(): Future<*> =
executorService.submit {
if (::process.isInitialized && process.isAlive) {
languageServer.shutdown().get(1, TimeUnit.SECONDS)
languageServer.exit()
if (process.isAlive) {
process.destroyForcibly()
}
fun shutdown() {
// LSP4j logs errors and rethrows - this is bad practice, and we don't need that log here, so we shut it up.
val lsp4jLogger = getLogger(RemoteEndpoint::class.java.name)
val previousLSP4jLogLevel = lsp4jLogger.level
lsp4jLogger.level = java.util.logging.Level.OFF
try {
val shouldShutdown = lsIsAlive()
executorService.submit { if (shouldShutdown) languageServer.shutdown().get(1, TimeUnit.SECONDS) }
} catch (ignored: Exception) {
// we don't care
} finally {
try {
if (lsIsAlive()) languageServer.exit()
} catch (ignore: Exception) {
// do nothing
} finally {
if (lsIsAlive()) process.destroyForcibly()
}
lsp4jLogger.level = previousLSP4jLogLevel
}
}

private fun lsIsAlive() = ::process.isInitialized && process.isAlive

private fun determineWorkspaceFolders(): List<WorkspaceFolder> {
val workspaceFolders = mutableSetOf<WorkspaceFolder>()
Expand Down
8 changes: 3 additions & 5 deletions src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt
Original file line number Diff line number Diff line change
Expand Up @@ -460,12 +460,10 @@ class SnykLanguageClient :
override fun showMessage(messageParams: MessageParams?) {
if (disposed) return
val project = ProjectUtil.getActiveProject()
if (project == null) {
logger.info(messageParams?.message)
return
}
when (messageParams?.type) {
MessageType.Error -> SnykBalloonNotificationHelper.showError(messageParams.message, project)
MessageType.Error -> {
SnykBalloonNotificationHelper.showError(messageParams.message, project)
}
MessageType.Warning -> SnykBalloonNotificationHelper.showWarn(messageParams.message, project)
MessageType.Info -> {
val notification = SnykBalloonNotificationHelper.showInfo(messageParams.message, project)
Expand Down
4 changes: 2 additions & 2 deletions src/main/kotlin/snyk/trust/TrustedProjects.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
package snyk.trust

import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.application.invokeAndWaitIfNeeded
import com.intellij.openapi.application.runInEdt
import com.intellij.openapi.components.service
import com.intellij.openapi.diagnostic.Logger
import com.intellij.openapi.project.Project
Expand Down Expand Up @@ -58,7 +58,7 @@ private fun confirmScanningUntrustedProject(project: Project): ScanUntrustedProj

var choice = ScanUntrustedProjectChoice.CANCEL

invokeAndWaitIfNeeded {
runInEdt {
val result = MessageDialogBuilder
.yesNo(title, message)
.icon(Messages.getWarningIcon())
Expand Down
6 changes: 5 additions & 1 deletion src/test/kotlin/io/snyk/plugin/TestUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ fun resetSettings(project: Project?) {
SnykProjectSettingsStateService(),
project
)
LanguageServerWrapper.getInstance().shutdown()
try {
LanguageServerWrapper.getInstance().shutdown()
} catch (ignore: Exception) {
// ignore
}
}

/** low level avoiding download the CLI file */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package io.snyk.plugin.services
import com.intellij.openapi.components.service
import com.intellij.testFramework.LightPlatformTestCase
import com.intellij.testFramework.PlatformTestUtil
import com.intellij.testFramework.replaceService
import io.mockk.every
import io.mockk.mockk
import io.mockk.mockkObject
Expand Down Expand Up @@ -69,7 +68,7 @@ class SnykTaskQueueServiceTest : LightPlatformTestCase() {
setupDummyCliFile()
val snykTaskQueueService = project.service<SnykTaskQueueService>()

snykTaskQueueService.scan(false)
snykTaskQueueService.scan()
PlatformTestUtil.dispatchAllInvocationEventsInIdeEventQueue()

assertTrue(snykTaskQueueService.getTaskQueue().isEmpty)
Expand All @@ -85,7 +84,7 @@ class SnykTaskQueueServiceTest : LightPlatformTestCase() {
every { isCliInstalled() } returns true

val snykTaskQueueService = project.service<SnykTaskQueueService>()
snykTaskQueueService.scan(false)
snykTaskQueueService.scan()

PlatformTestUtil.dispatchAllInvocationEventsInIdeEventQueue()
assertTrue(snykTaskQueueService.getTaskQueue().isEmpty)
Expand All @@ -99,7 +98,7 @@ class SnykTaskQueueServiceTest : LightPlatformTestCase() {
val snykTaskQueueService = project.service<SnykTaskQueueService>()
every { isCliInstalled() } returns true

snykTaskQueueService.scan(false)
snykTaskQueueService.scan()

PlatformTestUtil.dispatchAllInvocationEventsInIdeEventQueue()
assertTrue(snykTaskQueueService.getTaskQueue().isEmpty)
Expand Down Expand Up @@ -156,7 +155,7 @@ class SnykTaskQueueServiceTest : LightPlatformTestCase() {
every { isCliInstalled() } returns true
every { getIacService(project)?.scan() } returns fakeIacResult

snykTaskQueueService.scan(false)
snykTaskQueueService.scan()
PlatformTestUtil.dispatchAllInvocationEventsInIdeEventQueue()

assertEquals(fakeIacResult, getSnykCachedResults(project)?.currentIacResult)
Expand All @@ -178,7 +177,7 @@ class SnykTaskQueueServiceTest : LightPlatformTestCase() {

getSnykCachedResults(project)?.currentContainerResult = null

snykTaskQueueService.scan(false)
snykTaskQueueService.scan()
PlatformTestUtil.dispatchAllInvocationEventsInIdeEventQueue()

await().atMost(2, TimeUnit.SECONDS).until {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import io.mockk.CapturingSlot
import io.mockk.mockk
import io.mockk.unmockkAll
import io.mockk.verify
import io.snyk.plugin.getContentRootPaths
import io.snyk.plugin.toVirtualFile
import okio.Path.Companion.toPath
import org.eclipse.lsp4j.DidChangeConfigurationParams
Expand All @@ -21,18 +22,21 @@ import snyk.common.lsp.FolderConfigSettings
import snyk.common.lsp.LanguageServerSettings
import snyk.common.lsp.LanguageServerWrapper
import snyk.trust.WorkspaceTrustService
import snyk.trust.WorkspaceTrustSettings
import kotlin.io.path.absolutePathString


class BranchChooserComboBoxDialogTest : LightPlatform4TestCase() {
private val lsMock: LanguageServer = mockk(relaxed = true)
lateinit var folderConfig: FolderConfig
private lateinit var folderConfig: FolderConfig
lateinit var cut: BranchChooserComboBoxDialog

override fun setUp(): Unit {
super.setUp()
unmockkAll()
folderConfig = FolderConfig(project.basePath.toString(), "testBranch")
service<FolderConfigSettings>().addFolderConfig(folderConfig)
project.getContentRootPaths().forEach { service<WorkspaceTrustSettings>().addTrustedPath(it.root.absolutePathString())}
val languageServerWrapper = LanguageServerWrapper.getInstance()
languageServerWrapper.isInitialized = true
languageServerWrapper.languageServer = lsMock
Expand Down
Loading

0 comments on commit a790ab7

Please sign in to comment.