Skip to content

Commit 96a5c8f

Browse files
authored
Relaxed error handling (#471)
* Add GDB specific error classes to enable easier identification of specific errors coming from GDB to handle them differently than other errors. * Relax handling of errors for selected requests while running. Users cannot fix these and they often can't see them. Also, there is no good way to gracefully tell the IDE to stop a GUI update, and hence the can be considered as expected in such scenarios. * Skip CPU register and local variable reads while target running with aux gdb. This is to avoid expected errors in these scenarios. Neither registers nor local variables are expected to be reliably readable in this scenario. * Relax handling of command cancelled errors when adapter is existing session. These errors are usually provoked by a command queue flush when trying to gracefully disconnect. Hence, are kind of expected and don't need to be shown to the user. * Skip selected requests silently when debug adapter is not or no longer ready to reduce number of failing requests that can't succeed when the debug adapter is not ready yet/is no longer ready. Users can't change it, hence shouldn't be bothered with them. * Fix incorrect stop-gdbserver-skip reason text (launch connection, not attach) * Fix potential lockup with customResetRequest. The method wasn't sending a response if no customResetCommands were specified. A corner case not likely to be seen by users. But it hindered adding new tests. * Tests for relaxed error handling after disconnect --------- Signed-off-by: Jens Reinecke <jens.reinecke@arm.com>
1 parent cce8336 commit 96a5c8f

File tree

7 files changed

+635
-60
lines changed

7 files changed

+635
-60
lines changed

src/MIParser.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ const DEFAULT_CHARSET = 'utf-8';
2828
const FIXUPS_CHARSET = new Map<string, string>([['cp65001', 'utf-8']]);
2929

3030
export class MIParser {
31+
public static readonly CMD_QUEUE_CANCELLED =
32+
'MIParser command queue cancelled';
3133
/**
3234
* Encoding for GDB/MI cstring characters outside standard ASCII range.
3335
* Value has to match names known to TextDecoder API, see also
@@ -68,7 +70,7 @@ export class MIParser {
6870
entries.forEach((entry) => {
6971
// Call callback with error to reject command promise
7072
entry[1].callback('error', {
71-
msg: 'MI parser command queue cancelled',
73+
msg: MIParser.CMD_QUEUE_CANCELLED,
7274
});
7375
// Delete command by key
7476
delete this.commandQueue[entry[0]];

src/desktop/GDBTargetDebugSession.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
import { GDBBackendFactory } from './factories/GDBBackendFactory';
3232
import { GDBServerFactory } from './factories/GDBServerFactory';
3333
import { isProcessActive } from '../util/processes';
34+
import { GDBCommandCancelled } from '../gdb/errors';
3435

3536
// State of the Remote Target Debug Session
3637
enum SessionState {
@@ -175,6 +176,41 @@ export class GDBTargetDebugSession extends GDBDebugSession {
175176
};
176177
}
177178

179+
/**
180+
* Checks if debug adapter is connected and ready to process debug requests
181+
* driven by user interaction or other extensions.
182+
*
183+
* @return true if standard debug operation requests can be processed, false
184+
* otherwise.
185+
*/
186+
protected canRequestProceed(): boolean {
187+
if (this.sessionInfo.state !== SessionState.SESSION_READY) {
188+
return false;
189+
}
190+
return super.canRequestProceed();
191+
}
192+
193+
/**
194+
* Checks if an error would bring extra value to the user and if it
195+
* should be reported in the debug adapter protocol response.
196+
* Note: Call base class implementation last and after any explicit
197+
* return of value 'true'.
198+
* @param error The error to check.
199+
* @return true if the error should be reported as a debug adapter protocol
200+
* error response, false otherwise.
201+
*/
202+
protected shouldReportError(error: unknown): boolean {
203+
// GDBCommandCancelled occurrences for any GDB backend.
204+
if (
205+
error instanceof GDBCommandCancelled &&
206+
this.sessionInfo.state >= SessionState.EXITING
207+
) {
208+
// Exiting session, cancelling commands is expected for a graceful shutdown.
209+
return false;
210+
}
211+
return super.shouldReportError(error);
212+
}
213+
178214
/**
179215
* Validate the launch/attach request arguments and throw if they contain
180216
* an invalid combination.
@@ -761,7 +797,7 @@ export class GDBTargetDebugSession extends GDBDebugSession {
761797
return new Promise((resolve, reject) => {
762798
if (!this.gdbserver || !isProcessActive(this.gdbserver)) {
763799
const skipReason = this.launchGdbServer
764-
? `'attach' connection`
800+
? `'launch' connection`
765801
: 'already down';
766802
this.logGDBRemote(`skip stopping GDB server, ${skipReason}`);
767803
resolve(false);

src/gdb/GDBBackend.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ import { MIParser } from '../MIParser';
2828
import { compareVersions } from '../util/compareVersions';
2929
import { isProcessActive } from '../util/processes';
3030
import { NamedLogger } from '../namedLogger';
31+
import {
32+
GDBPipeError,
33+
GDBError,
34+
GDBUnknownResponse,
35+
GDBCommandCancelled,
36+
GDBThreadRunning,
37+
} from './errors';
3138

3239
// Expected console output for interpreter command 'show host-charset'
3340
// if setting is 'auto'.
@@ -333,7 +340,11 @@ export class GDBBackend extends events.EventEmitter implements IGDBBackend {
333340
// Reject command on pipe error, only way to recover from potential
334341
// race condition between command in flight and GDB (forced) shutdown.
335342
if (error) {
336-
reject(error);
343+
const gdbError =
344+
error.message === MIParser.CMD_QUEUE_CANCELLED
345+
? new GDBCommandCancelled(error, this.name)
346+
: new GDBPipeError(error, this.name);
347+
reject(gdbError);
337348
}
338349
};
339350
this.parser.queueCommand(
@@ -355,7 +366,16 @@ export class GDBBackend extends events.EventEmitter implements IGDBBackend {
355366
this.logger.verbose(
356367
`GDB command: ${token} ${command} failed with '${failure.message}'`
357368
);
358-
reject(failure);
369+
// Handle messages that contain info that thread is running
370+
// Note: Move to separate function if more cases need to be handled
371+
const threadRunningRegexp =
372+
/Selected thread is running/i;
373+
const gdbThreadRunning =
374+
threadRunningRegexp.test(failure.message);
375+
const gdbError = gdbThreadRunning
376+
? new GDBThreadRunning(failure, this.name)
377+
: new GDBError(failure, this.name);
378+
reject(gdbError);
359379
break;
360380
default:
361381
failure.message = `Unknown response ${resultClass}: ${JSON.stringify(
@@ -364,7 +384,9 @@ export class GDBBackend extends events.EventEmitter implements IGDBBackend {
364384
this.logger.verbose(
365385
`GDB command: ${token} ${command} failed with unknown response '${failure.message}'`
366386
);
367-
reject(failure);
387+
reject(
388+
new GDBUnknownResponse(failure, this.name)
389+
);
368390
}
369391
}
370392
);

0 commit comments

Comments
 (0)