You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by sh...@apache.org on 2023/06/20 18:02:26 UTC

[daffodil-vscode] branch main updated: Remove blocking dialogs for debugger errors

This is an automated email from the ASF dual-hosted git repository.

shanedell pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/daffodil-vscode.git


The following commit(s) were added to refs/heads/main by this push:
     new 6c5b0dd  Remove blocking dialogs for debugger errors
6c5b0dd is described below

commit 6c5b0dd09f5aa53acd431d582799a141e88a9921
Author: Shane Dell <sh...@gmail.com>
AuthorDate: Fri Jun 16 09:02:51 2023 -0400

    Remove blocking dialogs for debugger errors
    
    - When some errors happen in the debugger they would cause a large blocking dialog in the extension.
      - This didn't looks good and didn't allow you to quickly fix issues.
    - The errors now show in the terminal and small message in the bottom right corner.
    - Created a object for ErrorEvents with different case objects for error events to relay back to the extension.
    - Update extension to catch error events relayed from the debugger.
    - Create class for handling debugEvents so we don't have too many places changing onDidReceiveDebugSessionCustomEvent, on when absolutely necessary like in the hexview.
    
    Closes #649
---
 .../org.apache.daffodil.debugger.dap/DAPodil.scala | 54 ++++++++++++----------
 .../ErrorEvents.scala                              | 30 ++++++++++++
 src/adapter/activateDaffodilDebug.ts               |  5 ++
 src/adapter/daffodilEvent.ts                       | 37 +++++++++++++++
 src/infoset.ts                                     | 45 +++++-------------
 src/utils.ts                                       | 11 +++++
 6 files changed, 124 insertions(+), 58 deletions(-)

diff --git a/server/core/src/main/scala/org.apache.daffodil.debugger.dap/DAPodil.scala b/server/core/src/main/scala/org.apache.daffodil.debugger.dap/DAPodil.scala
index 6016c61..a3b6f54 100644
--- a/server/core/src/main/scala/org.apache.daffodil.debugger.dap/DAPodil.scala
+++ b/server/core/src/main/scala/org.apache.daffodil.debugger.dap/DAPodil.scala
@@ -49,6 +49,8 @@ trait DAPSession[Req, Res, Ev] {
 
   def sendResponse(response: Res): IO[Unit]
   def sendEvent(event: Ev): IO[Unit]
+  def abort(event: DebugEvent): IO[Unit]
+  def abort(event: DebugEvent, logMessage: String): IO[Unit]
 }
 
 object DAPSession {
@@ -63,6 +65,14 @@ object DAPSession {
 
       def sendEvent(event: DebugEvent): IO[Unit] =
         Logger[IO].info(show"<E $event") *> IO.blocking(server.sendEvent(event))
+
+      /** Send DebugEvent back to extension and exit session, ending debug */
+      def abort(event: DebugEvent): IO[Unit] =
+        sendEvent(event) *> sendEvent(new Events.TerminatedEvent())
+
+      /** Log error then send DebugEvent back to extension and exit session, ending debug */
+      def abort(event: DebugEvent, logMessage: String): IO[Unit] =
+        Logger[IO].error(logMessage) *> sendEvent(event) *> sendEvent(new Events.TerminatedEvent())
     }
 
   def resource(socket: Socket): Resource[IO, DAPSession[Request, Response, DebugEvent]] =
@@ -179,9 +189,7 @@ class DAPodil(
         disconnect(request, args)
       case extract(Command.EVALUATE, args: EvaluateArguments) =>
         eval(request, args)
-      case _ =>
-        Logger[IO].warn(show"unhandled request $request") *> session
-          .sendResponse(request.respondFailure())
+      case _ => session.abort(ErrorEvents.UnhandledRequest, show"unhandled request $request")
     }
 
   /** State.Uninitialized -> State.Initialized */
@@ -205,8 +213,10 @@ class DAPodil(
         debugee(request) match {
           case Left(errors) =>
             state.set(DAPodil.State.FailedToLaunch(request, errors, None)) *>
-              Logger[IO].warn(show"error parsing launch args: ${errors.mkString_(", ")}") *> session
-                .sendResponse(request.respondFailure(Some(show"error parsing launch args: ${errors.mkString_(", ")}")))
+              session.abort(
+                ErrorEvents.LaunchArgsParseError,
+                show"error parsing launch args: ${errors.mkString_("\n")}"
+              )
           case Right(dbgee) =>
             for {
               launched <- hotswap.swap {
@@ -219,10 +229,7 @@ class DAPodil(
                     DAPodil.State
                       .FailedToLaunch(request, NonEmptyList.of("couldn't launch from created debuggee"), Some(t))
                   ) *>
-                    Logger[IO].warn(t)(show"couldn't launch, request $request") *>
-                    session.sendResponse(
-                      request.respondFailure(Some(show"Couldn't launch Daffodil debugger: ${t.getMessage()}"))
-                    )
+                    session.abort(ErrorEvents.RequestError, show"couldn't launch, request $request")
                 case Right(launchedState) =>
                   state.set(launchedState) *>
                     session.sendResponse(request.respondSuccess())
@@ -242,7 +249,7 @@ class DAPodil(
             )
           )
         }
-      case _ => session.sendResponse(request.respondFailure())
+      case _ => session.abort(ErrorEvents.SourceError)
     }
 
   def source(request: Request, args: SourceArguments): IO[Unit] =
@@ -252,11 +259,11 @@ class DAPodil(
           .sourceContent(DAPodil.Source.Ref(args.sourceReference))
           .flatMap {
             case None =>
-              session.sendResponse(request.respondFailure(Some(s"unknown source ref ${args.sourceReference}")))
+              session.abort(ErrorEvents.SourceError)
             case Some(content) =>
               session.sendResponse(request.respondSuccess(new SourceResponseBody(content.value, "text/xml")))
           }
-      case _ => session.sendResponse(request.respondFailure())
+      case _ => session.abort(ErrorEvents.SourceError)
     }
 
   def setBreakpoints(request: Request, args: SetBreakpointArguments): IO[Unit] =
@@ -278,8 +285,7 @@ class DAPodil(
           _ <- session.sendResponse(response)
         } yield ()
       case _: DAPodil.State.FailedToLaunch =>
-        Logger[IO].warn("ignoring setBreakPoints request since previous launch failed") *>
-          session.sendResponse(request.respondFailure())
+        Logger[IO].warn("ignoring setBreakPoints request since previous launch failed")
       case s => DAPodil.InvalidState.raise(request, "Launched", s)
     }
 
@@ -358,11 +364,10 @@ class DAPodil(
           _ <- data.stack
             .findFrame(DAPodil.Frame.Id(args.frameId))
             .fold(
-              Logger[IO].warn(
-                s"couldn't find scopes for frame ${args.frameId}: ${data.stack.frames
-                    .map(f => f.id -> f.stackFrame.name)}"
-              ) *>
-                session.sendResponse(request.respondFailure(Some(s"couldn't find scopes for frame ${args.frameId}")))
+              session.abort(
+                ErrorEvents.ScopeNotFoundError,
+                s"couldn't find scopes for frame ${args.frameId}: ${data.stack.frames.map(f => f.id -> f.stackFrame.name)}"
+              )
             ) { frame =>
               session.sendResponse(
                 request.respondSuccess(new Responses.ScopesResponseBody(frame.scopes.map(_.toDAP()).asJava))
@@ -381,11 +386,10 @@ class DAPodil(
           _ <- data.stack
             .variables(DAPodil.VariablesReference(args.variablesReference))
             .fold(
-              Logger[IO]
-                .warn(
-                  show"couldn't find variablesReference ${args.variablesReference} in stack ${data}"
-                ) *> // TODO: handle better
-                session.sendResponse(request.respondFailure())
+              session.abort(
+                ErrorEvents.UnexpectedError,
+                show"couldn't find variablesReference ${args.variablesReference} in stack ${data}"
+              )
             )(variables =>
               session.sendResponse(request.respondSuccess(new Responses.VariablesResponseBody(variables.asJava)))
             )
@@ -399,7 +403,7 @@ class DAPodil(
         for {
           variable <- launched.debugee.eval(args)
           _ <- variable match {
-            case None => session.sendResponse(request.respondFailure())
+            case None => session.abort(ErrorEvents.UnexpectedError)
             case Some(v) =>
               session.sendResponse(request.respondSuccess(new Responses.EvaluateResponseBody(v.value, 0, null, 0)))
           }
diff --git a/server/core/src/main/scala/org.apache.daffodil.debugger.dap/ErrorEvents.scala b/server/core/src/main/scala/org.apache.daffodil.debugger.dap/ErrorEvents.scala
new file mode 100644
index 0000000..1861cfd
--- /dev/null
+++ b/server/core/src/main/scala/org.apache.daffodil.debugger.dap/ErrorEvents.scala
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil.debugger.dap
+
+import com.microsoft.java.debug.core.protocol.Events.DebugEvent
+
+/** Case classes for errors that we want to relay back to the extension */
+object ErrorEvents {
+  case object UnexpectedError extends DebugEvent("daffodil.error.unexpected")
+  case object UnhandledRequest extends DebugEvent("daffodil.error.requestunhandled")
+  case object LaunchArgsParseError extends DebugEvent("daffodil.error.launchargparse")
+  case object RequestError extends DebugEvent("daffodil.error.request")
+  case object SourceError extends DebugEvent("daffodil.error.source")
+  case object ScopeNotFoundError extends DebugEvent("daffodil.error.scopenotfound")
+}
diff --git a/src/adapter/activateDaffodilDebug.ts b/src/adapter/activateDaffodilDebug.ts
index a1c57b2..8eab68c 100644
--- a/src/adapter/activateDaffodilDebug.ts
+++ b/src/adapter/activateDaffodilDebug.ts
@@ -24,6 +24,7 @@ import { getConfig, getCurrentConfig, setCurrentConfig } from '../utils'
 import { DaffodilDebugSession } from './daffodilDebug'
 import { FileAccessor } from './daffodilRuntime'
 import { TDMLConfig } from '../classes/tdmlConfig'
+import { handleDebugEvent } from './daffodilEvent'
 
 /** Method to file path for program and data
  * Details:
@@ -378,6 +379,10 @@ export function activateDaffodilDebug(
     })
   )
 
+  context.subscriptions.push(
+    vscode.debug.onDidReceiveDebugSessionCustomEvent(handleDebugEvent)
+  )
+
   dfdlLang.activate(context)
   infoset.activate(context)
   dataEditClient.activate(context)
diff --git a/src/adapter/daffodilEvent.ts b/src/adapter/daffodilEvent.ts
new file mode 100644
index 0000000..45ddeaa
--- /dev/null
+++ b/src/adapter/daffodilEvent.ts
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as vscode from 'vscode'
+import * as fs from 'fs'
+
+import * as daf from '../daffodil'
+import { ensureFile, tmpFile } from '../utils'
+
+export function handleDebugEvent(e: vscode.DebugSessionCustomEvent) {
+  switch (e.event) {
+    case daf.infosetEvent:
+      let update: daf.InfosetEvent = e.body
+      let path = ensureFile(tmpFile(e.session.id))
+      fs.copyFileSync(path, `${path}.prev`)
+      fs.writeFileSync(path, update.content)
+      break
+    // this allows for any error event to be caught in this case
+    case e.event.startsWith('daffodil.error') ? e.event : '':
+      vscode.window.showErrorMessage(`debugger ${e.event}`)
+      break
+  }
+}
diff --git a/src/infoset.ts b/src/infoset.ts
index 8dcc2f2..d1d338b 100644
--- a/src/infoset.ts
+++ b/src/infoset.ts
@@ -15,13 +15,16 @@
  * limitations under the License.
  */
 
-import { tmpdir } from 'os'
 import * as vscode from 'vscode'
 import * as daf from './daffodil'
 import * as fs from 'fs'
-import { InfosetEvent } from './daffodil'
 import { Uri } from 'vscode'
-import { onDebugStartDisplay, getCurrentConfig } from './utils'
+import {
+  onDebugStartDisplay,
+  getCurrentConfig,
+  ensureFile,
+  tmpFile,
+} from './utils'
 
 // Function to display an infomation message that the infoset file has been created
 // If the user wishes to open the file then they may click the 'Open' button
@@ -69,7 +72,7 @@ export async function activate(ctx: vscode.ExtensionContext) {
   ctx.subscriptions.push(
     vscode.debug.onDidTerminateDebugSession(async (s) => {
       if (sid !== undefined) {
-        let path = tmp(sid)
+        let path = tmpFile(sid)
         fs.rmSync(`${path}`, { force: true })
         fs.rmSync(`${path}.prev`, { force: true })
       }
@@ -77,9 +80,7 @@ export async function activate(ctx: vscode.ExtensionContext) {
       await openInfosetFilePrompt()
     })
   )
-  ctx.subscriptions.push(
-    vscode.debug.onDidReceiveDebugSessionCustomEvent(handleEvent)
-  )
+
   ctx.subscriptions.push(
     vscode.workspace.registerTextDocumentContentProvider(
       'daffodil:infoset',
@@ -90,7 +91,7 @@ export async function activate(ctx: vscode.ExtensionContext) {
   ctx.subscriptions.push(
     vscode.commands.registerCommand('infoset.display', async () => {
       if (sid !== undefined) {
-        let path = ensure(tmp(sid))
+        let path = ensureFile(tmpFile(sid))
         doc = await vscode.workspace.openTextDocument(path)
         await vscode.window.showTextDocument(doc, {
           viewColumn: vscode.ViewColumn.Two,
@@ -110,7 +111,7 @@ export async function activate(ctx: vscode.ExtensionContext) {
             placeHolder: 'Save infoset as:',
           })
           if (dest) {
-            fs.copyFile(tmp(sid), dest, async () => {
+            fs.copyFile(tmpFile(sid), dest, async () => {
               const choice = await vscode.window.showInformationMessage(
                 `Wrote infoset to ${dest}`,
                 'View',
@@ -139,8 +140,8 @@ export async function activate(ctx: vscode.ExtensionContext) {
   ctx.subscriptions.push(
     vscode.commands.registerCommand('infoset.diff', async () => {
       if (sid !== undefined) {
-        let path = ensure(tmp(sid))
-        let prev = ensure(`${path}.prev`)
+        let path = ensureFile(tmpFile(sid))
+        let prev = ensureFile(`${path}.prev`)
         vscode.commands.executeCommand(
           'vscode.diff',
           Uri.parse(prev),
@@ -153,17 +154,6 @@ export async function activate(ctx: vscode.ExtensionContext) {
   )
 }
 
-async function handleEvent(e: vscode.DebugSessionCustomEvent) {
-  switch (e.event) {
-    case daf.infosetEvent:
-      let update: InfosetEvent = e.body
-      let path = ensure(tmp(e.session.id))
-      fs.copyFileSync(path, `${path}.prev`)
-      fs.writeFileSync(path, update.content)
-      break
-  }
-}
-
 const fileInfosetProvider = new (class
   implements vscode.TextDocumentContentProvider
 {
@@ -171,14 +161,3 @@ const fileInfosetProvider = new (class
     return fs.readFileSync(uri.path).toString()
   }
 })()
-
-function tmp(sid: string): string {
-  return `${tmpdir()}/infoset-${sid}.${getCurrentConfig().infosetFormat}`
-}
-
-function ensure(path: string): string {
-  if (!fs.existsSync(path)) {
-    fs.writeFileSync(path, '')
-  }
-  return path
-}
diff --git a/src/utils.ts b/src/utils.ts
index 6e3408e..d2d2e27 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -348,3 +348,14 @@ export async function findExistingOmegaEditServer(
   }
   return ret
 }
+
+export function tmpFile(sid: string): string {
+  return `${os.tmpdir()}/infoset-${sid}.${getCurrentConfig().infosetFormat}`
+}
+
+export function ensureFile(path: string): string {
+  if (!fs.existsSync(path)) {
+    fs.writeFileSync(path, '')
+  }
+  return path
+}