always report gdb launch errors during launch (fixes #1096)

[malsyned]

I'm not well-enough versed in JavaScript async & Promises and the way they interact to be sure about this, but as far as I can tell, the problem seems to be that if the Node interpreter runs out of events to process and still has some rejected promises with no reject handlers registered, it considers that an error and crashes with a stack trace. This patch makes sure that control is never handed back to the event loop with no reject handler registered for gdbPromise.

I'm not 100% confident in my diagnosis or solution, but it works for me, both with and without the breakpoint I described in #1096. If someone with more understanding of the interaction between async/await and Promises wants to steer me in a different direction, I'm interested.

[haneefdm]

I am in the middle of something that will complicate this merge. This is a good solution and I will merge the changes when things are stable.

However, lint is not passing. You can do a npm run lint.

[haneefdm]

There may be another problem. I see a stack trace in #1096. I think we should look at why that is happening. We may be masking that bug by doing what you did in this PR. We should merge this change but also look at why we are crashing.

If you can help, that would be great.

[malsyned]

There may be another problem. I see a stack trace in #1096. I think we should look at why that is happening. We may be masking that bug by doing what you did in this PR. We should merge this change but also look at why we are crashing.

If you can help, that would be great.

Happy to help. I don't think the stack trace is evidence of anything else broken, but I agree that it would be nice to get rid of it.

It's happening because:

1. GDB replies to -file-exec-and-symbols with an error response.
2. The errReport() closure in sendOneCommand() calls reject() with a new MIError
3. The try/catch block that starts a little before await gdbPromise catches this MIError which is thrown either from the await line or the new throw right before it that this PR adds.

I think this is all working well enough. The problem is that the catch block in question is trying to catch all errors from a big block of code, and is written fairly generically to attach a stack trace to whatever is thrown. Does it make sense to check specifically for MIError and send a launchErrorResponse that doesn't include the stack trace?

Something like:

@@ -688,9 +690,14 @@ export class GDBDebugSession extends LoggingDebugSession {
                             commands.push(...this.args.postLaunchCommands.map(COMMAND_MAP));
                         }
                     } catch (err) {
-                        const msg = err.toString() + '\n' + err.stack.toString();
-                        this.sendEvent(new TelemetryEvent('Error', 'Launching GDB', `Failed to generate gdb commands: ${msg}`));
-                        this.launchErrorResponse(response, 104, `Failed to generate gdb commands: ${msg}`);
+                        let msg: string;
+                        if (err instanceof MIError) {
+                            msg = `Failed to initialize GDB: ${err.message}\n\ncommand: ${err.source}`;
+                        } else {
+                            msg = 'Failed to generate gdb commands: ' + err.toString() + '\n' + err.stack.toString();
+                        }
+                        this.sendEvent(new TelemetryEvent('Error', 'Launching GDB', msg));
+                        this.launchErrorResponse(response, 104, msg);
                         return doResolve();
                     }

Giving an error popup like this:

I don't have strong opinions about the exact wording of the error. I included the command that resulted in the error, but I'm not sure that actually adds anything of value. I'm also not completely clear on the significance of the 104 error code, or whether it's appropriate to use it for both kinds of messages there. Can you give any guidance on that?