Merge pull request #29 from dprint/timeout-can-format

Timeout canFormat in EditorServiceV4 so it cannot lock up the UI.

CHANGELOG.md

@@ -3,6 +3,7 @@
 # dprint-intellij-plugin Changelog
 
 ## [Unreleased]
+- Fix intermittent lock up when running format
 
 ## [0.3.1]
 - Fix versioning to allow for 2021.3.x installs
@@ -32,4 +33,4 @@
 - Fix issue where the inability to parse the schema would stop a project form opening.
 
 ## [0.1.2]
-- Release first public version of the plugin.
+- Release first public version of the plugin.

gradle.properties

@@ -2,7 +2,7 @@
 # -> https://plugins.jetbrains.com/docs/intellij/intellij-artifacts.html
 pluginGroup=com.dprint.intellij.plugin
 pluginName=dprint-intellij-plugin
-pluginVersion=0.3.1
+pluginVersion=0.3.2
 # See https://plugins.jetbrains.com/docs/intellij/build-number-ranges.html
 # for insight into build numbers and IntelliJ Platform versions.
 pluginSinceBuild=213

src/main/kotlin/com/dprint/formatter/DprintExternalFormatter.kt

@@ -12,9 +12,12 @@ import com.intellij.openapi.components.service
 import com.intellij.openapi.diagnostic.logger
 import com.intellij.psi.PsiFile
 import java.util.concurrent.CompletableFuture
+import java.util.concurrent.TimeUnit
+import java.util.concurrent.TimeoutException
 
 private val LOGGER = logger<DprintExternalFormatter>()
 private const val NAME = "dprintfmt"
+private const val CAN_FORMAT_TIMEOUT = 4L
 
 class DprintExternalFormatter : AsyncDocumentFormattingService() {
     override fun getFeatures(): MutableSet<FormattingService.Feature> {
@@ -24,10 +27,28 @@ class DprintExternalFormatter : AsyncDocumentFormattingService() {
     }
 
     override fun canFormat(file: PsiFile): Boolean {
-        val editorService = file.project.service<EditorServiceManager>().maybeGetEditorService()
-        return editorService != null && file.virtualFile != null &&
-            file.project.service<ProjectConfiguration>().state.enabled &&
-            editorService.canFormat(file.virtualFile.path)
+        try {
+            val future = CompletableFuture.supplyAsync {
+                val editorService = file.project.service<EditorServiceManager>().maybeGetEditorService()
+                editorService != null &&
+                    file.virtualFile != null &&
+                    file.project.service<ProjectConfiguration>().state.enabled &&
+                    editorService.canFormat(file.virtualFile.path)
+            }.orTimeout(CAN_FORMAT_TIMEOUT, TimeUnit.SECONDS)
+            return future.get()
+        } catch (e: TimeoutException) {
+            LogUtils.error(
+                Bundle.message(
+                    "editor.service.timed.out.checking.if.can.format",
+                    file.virtualFile?.path ?: "Unknown file path"
+                ),
+                e,
+                file.project,
+                LOGGER
+            )
+        }
+
+        return false
     }
 
     override fun createFormattingTask(formattingRequest: AsyncFormattingRequest): FormattingTask? {
@@ -75,14 +96,13 @@ class DprintExternalFormatter : AsyncDocumentFormattingService() {
                                 nextFuture.complete(nextResult)
                             }
                             if (resultContent != null) {
-                                formattingId =
-                                    editorService.fmt(
-                                        path,
-                                        resultContent,
-                                        range.startOffset,
-                                        range.endOffset,
-                                        nextHandler
-                                    )
+                                formattingId = editorService.fmt(
+                                    path,
+                                    resultContent,
+                                    range.startOffset,
+                                    range.endOffset,
+                                    nextHandler
+                                )
                             }
                             nextFuture.get()
                         }
@@ -106,12 +126,12 @@ class DprintExternalFormatter : AsyncDocumentFormattingService() {
                     return
                 }
 
-                result.formattedContent?.let {
-                    formattingRequest.onTextReady(it)
-                }
-
-                result.error?.let {
-                    formattingRequest.onError(Bundle.message("formatting.error"), it)
+                val error = result.error
+                if (error != null) {
+                    formattingRequest.onError(Bundle.message("formatting.error"), error)
+                } else {
+                    // If the result is a no op it will be null, in which case we pass the original content back in
+                    formattingRequest.onTextReady(result.formattedContent ?: content)
                 }
             }
 

src/main/kotlin/com/dprint/services/FormatterService.kt

@@ -129,6 +129,11 @@ class FormatterService(private val project: Project) {
             override fun run(indicator: ProgressIndicator) {
                 handler(indicator)
             }
+
+            override fun onCancel() {
+                val editorServiceInstance = editorServiceManager.maybeGetEditorService()
+                editorServiceInstance?.initialiseEditorService()
+            }
         }
         this.formatTaskQueue.run(task)
     }

src/main/kotlin/com/dprint/services/editorservice/EditorService.kt

@@ -26,6 +26,8 @@ interface EditorService : Disposable {
      * @param filePath The path of the file being formatted. This is needed so the correct dprint configuration file
      * located.
      * @param content The content of the file as a string. This is formatted via Dprint and returned via the result.
+     * @param onFinished A callback that is called when the formatting job has finished. The only param to this callback
+     * will be the result of the formatting job. The class providing this should handle timeouts themselves.
      * @return A result object containing the formatted content is successful or an error.
      */
     fun fmt(filePath: String, content: String, onFinished: (FormatResult) -> Unit): Int? {

src/main/kotlin/com/dprint/services/editorservice/v4/EditorServiceV4.kt

@@ -17,9 +17,6 @@ private const val FORMAT_COMMAND = 2
 
 private val LOGGER = logger<EditorServiceV4>()
 
-/**
- * A command line wrapper to run Dprint.
- */
 @Service
 class EditorServiceV4(private val project: Project) : EditorService {
     private var editorProcess = EditorProcess(project)
@@ -44,28 +41,28 @@ class EditorServiceV4(private val project: Project) : EditorService {
         editorProcess.destroy()
     }
 
+    @Synchronized
     override fun canFormat(filePath: String): Boolean {
         var status = 0
         LogUtils.info(Bundle.message("formatting.checking.can.format", filePath), project, LOGGER)
 
         try {
-            synchronized(this) {
-                editorProcess.writeInt(CHECK_COMMAND)
-                editorProcess.writeString(filePath)
-                editorProcess.writeSuccess()
-
-                // https://github.com/dprint/dprint/blob/main/docs/editor-extension-development.md
-                // this command sequence returns 1 if the file can be formatted
-                status = editorProcess.readInt()
-                editorProcess.readAndAssertSuccess()
-            }
+            editorProcess.writeInt(CHECK_COMMAND)
+            editorProcess.writeString(filePath)
+            editorProcess.writeSuccess()
+
+            // https://github.com/dprint/dprint/blob/main/docs/editor-extension-development.md
+            // this command sequence returns 1 if the file can be formatted
+            status = editorProcess.readInt()
+            editorProcess.readAndAssertSuccess()
         } catch (e: ProcessUnavailableException) {
             LogUtils.error(
                 Bundle.message("editor.service.unable.to.determine.if.can.format", filePath),
                 e,
                 project,
                 LOGGER
             )
+            initialiseEditorService()
         }
 
         val result = status == 1
@@ -81,48 +78,57 @@ class EditorServiceV4(private val project: Project) : EditorService {
         return false
     }
 
-    /**
-     * This runs dprint using the editor service with the supplied file path and content as stdin.
-     * @param filePath The path of the file being formatted. This is needed so the correct dprint configuration file
-     * located.
-     * @param content The content of the file as a string. This is formatted via Dprint and returned via the result.
-     * @return A result object containing the formatted content is successful or an error.
-     */
+    @Synchronized
     override fun fmt(filePath: String, content: String, onFinished: (FormatResult) -> Unit): Int? {
         val result = FormatResult()
 
         LogUtils.info(Bundle.message("formatting.file", filePath), project, LOGGER)
-
         try {
-            synchronized(this) {
-                editorProcess.writeInt(FORMAT_COMMAND)
-                editorProcess.writeString(filePath)
-                editorProcess.writeString(content)
-                editorProcess.writeSuccess()
-
-                when (editorProcess.readInt()) {
-                    0 -> Unit // no-op as content didn't change
-                    1 -> result.formattedContent = editorProcess.readString()
-                    2 -> {
-                        val error = editorProcess.readString()
-                        result.error = error
-                        LogUtils.warn(
-                            Bundle.message("editor.service.format.failed", filePath, error),
-                            project,
-                            LOGGER
-                        )
-                    }
+            editorProcess.writeInt(FORMAT_COMMAND)
+            editorProcess.writeString(filePath)
+            editorProcess.writeString(content)
+            editorProcess.writeSuccess()
+
+            when (editorProcess.readInt()) {
+                0 -> {
+                    LogUtils.info(
+                        Bundle.message("editor.service.format.not.needed", filePath),
+                        project,
+                        LOGGER
+                    )
+                } // no-op as content didn't change
+                1 -> {
+                    result.formattedContent = editorProcess.readString()
+                    LogUtils.info(
+                        Bundle.message("editor.service.format.succeeded", filePath),
+                        project,
+                        LOGGER
+                    )
+                }
+                2 -> {
+                    val error = editorProcess.readString()
+                    result.error = error
+                    LogUtils.warn(
+                        Bundle.message("editor.service.format.failed", filePath, error),
+                        project,
+                        LOGGER
+                    )
                 }
-
-                editorProcess.readAndAssertSuccess()
             }
+
+            editorProcess.readAndAssertSuccess()
         } catch (e: ProcessUnavailableException) {
             LogUtils.error(
-                Bundle.message("editor.service.unable.to.determine.if.can.format", filePath),
+                Bundle.message(
+                    "editor.service.format.failed.internal",
+                    filePath,
+                    e.message ?: "Process unavailable"
+                ),
                 e,
                 project,
                 LOGGER
             )
+            initialiseEditorService()
         }
 
         onFinished(result)

src/main/kotlin/com/dprint/services/editorservice/v5/EditorServiceV5.kt

@@ -19,7 +19,7 @@ import kotlin.concurrent.thread
 
 private val LOGGER = logger<EditorServiceV5>()
 private const val FORCE_DESTROY_DELAY = 1000L
-private const val FORMATTING_TIMEOUT_SECONDS = 10L
+private const val CAN_FORMAT_TIMEOUT_SECONDS = 4L
 
 @Service
 class EditorServiceV5(val project: Project) : EditorService {
@@ -78,7 +78,7 @@ class EditorServiceV5(val project: Project) : EditorService {
         editorProcess.writeBuffer(message.build())
 
         return try {
-            val result = future.get(FORMATTING_TIMEOUT_SECONDS, TimeUnit.SECONDS)
+            val result = future.get(CAN_FORMAT_TIMEOUT_SECONDS, TimeUnit.SECONDS)
             if (result.type == MessageType.CanFormatResponse && result.data is Boolean) {
                 result.data
             } else if (result.type == MessageType.ErrorResponse && result.data is String) {
@@ -129,6 +129,11 @@ class EditorServiceV5(val project: Project) : EditorService {
         val handler: (PendingMessages.Result) -> Unit = {
             val formatResult = FormatResult()
             if (it.type == MessageType.FormatFileResponse && it.data is String?) {
+                val successMessage = when (it.data) {
+                    null -> Bundle.message("editor.service.format.not.needed", filePath)
+                    else -> Bundle.message("editor.service.format.succeeded", filePath)
+                }
+                LogUtils.info(successMessage, project, LOGGER)
                 formatResult.formattedContent = it.data
             } else if (it.type == MessageType.ErrorResponse && it.data is String) {
                 LogUtils.warn(Bundle.message("editor.service.format.failed", filePath, it.data), project, LOGGER)

src/main/resources/messages/Bundle.properties

@@ -23,6 +23,9 @@ editor.service.created.formatting.task=Created formatting task for {0} with id {
 editor.service.destroy=Destroying {0}
 editor.service.format.check.failed=Dprint failed to check of the file {0} can be formatted due to:\n{1}
 editor.service.format.failed=Dprint failed to format the file {0} due to:\n{1}
+editor.service.format.failed.internal=Failed to format the file {0} due to:\n{1}
+editor.service.format.not.needed=No need to format {0}
+editor.service.format.succeeded=Successfully formatted {0}
 editor.service.incorrect.message.size=Incorrect message size, expected {0} and got {1}
 editor.service.initialize=Initializing {0}
 editor.service.read.failed=Failed to read stdout of the editor service
@@ -31,6 +34,7 @@ editor.service.shutting.down=Shutting down current editor service
 editor.service.started.stdout.listener=Started stdout listener
 editor.service.starting.working.dir=Starting editor service with executable {0}, config {1}. No working dir found.
 editor.service.starting=Starting editor service with executable {0}, config {1} and working directory {2}.
+editor.service.timed.out.checking.if.can.format=Timed out while checking if the file {0} can be formatted.
 editor.service.unable.to.determine.if.can.format=Unable to determine if the file {0} can be formatted.
 editor.service.unsupported.message.type=Received unsupported message type {0}.
 error.config.path=Unable to retrieve a valid dprint configuration path.