Skip to content

Commit 6daf0af

Browse files
author
Akos Kitta
committed
fix: propagate monitor errors to the frontend
Closes #1508 Signed-off-by: Akos Kitta <[email protected]>
1 parent 9b49712 commit 6daf0af

File tree

8 files changed

+241
-133
lines changed

8 files changed

+241
-133
lines changed

Diff for: ‎arduino-ide-extension/src/browser/monitor-manager-proxy-client-impl.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
ApplicationError,
23
CommandRegistry,
34
Disposable,
45
Emitter,
@@ -167,7 +168,13 @@ export class MonitorManagerProxyClientImpl
167168
const { selectedBoard, selectedPort } =
168169
this.boardsServiceProvider.boardsConfig;
169170
if (!selectedBoard || !selectedBoard.fqbn || !selectedPort) return;
170-
await this.server().startMonitor(selectedBoard, selectedPort, settings);
171+
try {
172+
await this.server().startMonitor(selectedBoard, selectedPort, settings);
173+
} catch (err) {
174+
this.messageService.error(
175+
ApplicationError.is(err) ? err.message : String(err)
176+
);
177+
}
171178
}
172179

173180
getCurrentSettings(board: Board, port: Port): Promise<MonitorSettings> {

Diff for: ‎arduino-ide-extension/src/browser/monitor-model.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export class MonitorModel implements FrontendApplicationContribution {
3939
this._darkTheme = false;
4040
this._wsPort = 0;
4141
this._serialPort = '';
42-
this._connected = true;
42+
this._connected = false;
4343

4444
this.onChangeEmitter = new Emitter<
4545
MonitorModel.State.Change<keyof MonitorModel.State>

Diff for: ‎arduino-ide-extension/src/common/protocol/monitor-service.ts

+125-21
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Event, JsonRpcServer } from '@theia/core';
1+
import { ApplicationError, Event, JsonRpcServer, nls } from '@theia/core';
22
import {
33
PluggableMonitorSettings,
44
MonitorSettings,
@@ -46,7 +46,7 @@ export interface PluggableMonitorSetting {
4646
readonly id: string;
4747
// A human-readable label of the setting (to be displayed on the GUI)
4848
readonly label: string;
49-
// The setting type (at the moment only "enum" is avaiable)
49+
// The setting type (at the moment only "enum" is available)
5050
readonly type: string;
5151
// The values allowed on "enum" types
5252
readonly values: string[];
@@ -72,24 +72,128 @@ export namespace Monitor {
7272
};
7373
}
7474

75-
export interface Status {}
76-
export type OK = Status;
77-
export interface ErrorStatus extends Status {
78-
readonly message: string;
75+
export const MonitorErrorCodes = {
76+
ConnectionFailed: 6001,
77+
NotConnected: 6002,
78+
AlreadyConnected: 6003,
79+
MissingConfiguration: 6004,
80+
} as const;
81+
82+
export const ConnectionFailedError = createMonitorError(
83+
MonitorErrorCodes.ConnectionFailed
84+
);
85+
export const NotConnectedError = createMonitorError(
86+
MonitorErrorCodes.NotConnected
87+
);
88+
export const AlreadyConnectedError = createMonitorError(
89+
MonitorErrorCodes.AlreadyConnected
90+
);
91+
export const MissingConfigurationError = createMonitorError(
92+
MonitorErrorCodes.MissingConfiguration
93+
);
94+
95+
export function createConnectionFailedError(
96+
port: Port,
97+
details?: string
98+
): ApplicationError<number, PortDescriptor> {
99+
const { protocol, protocolLabel, address, addressLabel } = port;
100+
return ConnectionFailedError(
101+
connectionFailedErrorLabel(addressLabel, protocolLabel, details),
102+
{ protocol, address }
103+
);
79104
}
80-
export namespace Status {
81-
export function isOK(status: Status & { message?: string }): status is OK {
82-
return !!status && typeof status.message !== 'string';
83-
}
84-
export const OK: OK = {};
85-
export const NOT_CONNECTED: ErrorStatus = { message: 'Not connected.' };
86-
export const ALREADY_CONNECTED: ErrorStatus = {
87-
message: 'Already connected.',
88-
};
89-
export const CONFIG_MISSING: ErrorStatus = {
90-
message: 'Serial Config missing.',
91-
};
92-
export const UPLOAD_IN_PROGRESS: ErrorStatus = {
93-
message: 'Upload in progress.',
94-
};
105+
106+
export function createNotConnectedError(
107+
port: Port
108+
): ApplicationError<number, PortDescriptor> {
109+
const { protocol, protocolLabel, address, addressLabel } = port;
110+
return NotConnectedError(
111+
notConnectedErrorLabel(addressLabel, protocolLabel),
112+
{ protocol, address }
113+
);
114+
}
115+
116+
export function createAlreadyConnectedError(
117+
port: Port
118+
): ApplicationError<number, PortDescriptor> {
119+
const { protocol, protocolLabel, address, addressLabel } = port;
120+
return AlreadyConnectedError(
121+
alreadyConnectedErrorLabel(addressLabel, protocolLabel),
122+
{ protocol, address }
123+
);
124+
}
125+
126+
export function createMissingConfigurationError(
127+
port: Port
128+
): ApplicationError<number, PortDescriptor> {
129+
const { protocol, protocolLabel, address, addressLabel } = port;
130+
return MissingConfigurationError(
131+
missingConfigurationErrorLabel(addressLabel, protocolLabel),
132+
{ protocol, address }
133+
);
134+
}
135+
136+
/**
137+
* Bare minimum representation of a port. Supports neither UI labels nor properties.
138+
*/
139+
interface PortDescriptor {
140+
readonly protocol: string;
141+
readonly address: string;
142+
}
143+
144+
function createMonitorError(
145+
code: number
146+
): ApplicationError.Constructor<number, PortDescriptor> {
147+
return ApplicationError.declare(
148+
code,
149+
(message: string, data: PortDescriptor) => ({ data, message })
150+
);
151+
}
152+
153+
export function connectionFailedErrorLabel(
154+
addressLabel: string,
155+
protocolLabel: string,
156+
details?: string
157+
): string {
158+
const formattedDetails = details ? `: ${details}.` : '.';
159+
return nls.localize(
160+
'arduino/monitor/connectionFailedError',
161+
'Could not connect to {0} {1} port{2}',
162+
addressLabel,
163+
protocolLabel,
164+
formattedDetails
165+
);
166+
}
167+
export function notConnectedErrorLabel(
168+
addressLabel: string,
169+
protocolLabel: string
170+
): string {
171+
return nls.localize(
172+
'arduino/monitor/notConnectedError',
173+
'Not connected to {0} {1} port.',
174+
addressLabel,
175+
protocolLabel
176+
);
177+
}
178+
export function alreadyConnectedErrorLabel(
179+
addressLabel: string,
180+
protocolLabel: string
181+
): string {
182+
return nls.localize(
183+
'arduino/monitor/alreadyConnectedError',
184+
'Could not connect to {0} {1} port. Already connected.',
185+
addressLabel,
186+
protocolLabel
187+
);
188+
}
189+
export function missingConfigurationErrorLabel(
190+
addressLabel: string,
191+
protocolLabel: string
192+
): string {
193+
return nls.localize(
194+
'arduino/monitor/missingConfigurationError',
195+
'Could not connect to {0} {1} port. The monitor configuration is missing.',
196+
addressLabel,
197+
protocolLabel
198+
);
95199
}

Diff for: ‎arduino-ide-extension/src/node/core-service-impl.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
UploadUsingProgrammerResponse,
2424
} from './cli-protocol/cc/arduino/cli/commands/v1/upload_pb';
2525
import { ResponseService } from '../common/protocol/response-service';
26-
import { OutputMessage, Port, Status } from '../common/protocol';
26+
import { OutputMessage, Port } from '../common/protocol';
2727
import { ArduinoCoreServiceClient } from './cli-protocol/cc/arduino/cli/commands/v1/commands_grpc_pb';
2828
import { Port as RpcPort } from './cli-protocol/cc/arduino/cli/commands/v1/port_pb';
2929
import { ApplicationError, CommandService, Disposable, nls } from '@theia/core';
@@ -392,7 +392,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
392392
}: {
393393
fqbn?: string | undefined;
394394
port?: Port | undefined;
395-
}): Promise<Status> {
395+
}): Promise<void> {
396396
this.boardDiscovery.setUploadInProgress(false);
397397
return this.monitorManager.notifyUploadFinished(fqbn, port);
398398
}

Diff for: ‎arduino-ide-extension/src/node/monitor-manager-proxy-impl.ts

+2-6
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { inject, injectable } from '@theia/core/shared/inversify';
22
import {
33
MonitorManagerProxy,
44
MonitorManagerProxyClient,
5-
Status,
65
} from '../common/protocol';
76
import { Board, Port } from '../common/protocol';
87
import { MonitorManager } from './monitor-manager';
@@ -41,11 +40,8 @@ export class MonitorManagerProxyImpl implements MonitorManagerProxy {
4140
await this.changeMonitorSettings(board, port, settings);
4241
}
4342

44-
const connectToClient = (status: Status) => {
45-
if (status === Status.ALREADY_CONNECTED || status === Status.OK) {
46-
// Monitor started correctly, connect it with the frontend
47-
this.client.connect(this.manager.getWebsocketAddressPort(board, port));
48-
}
43+
const connectToClient = async () => {
44+
this.client.connect(this.manager.getWebsocketAddressPort(board, port));
4945
};
5046
return this.manager.startMonitor(board, port, connectToClient);
5147
}

Diff for: ‎arduino-ide-extension/src/node/monitor-manager.ts

+20-11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { ILogger } from '@theia/core';
22
import { inject, injectable, named } from '@theia/core/shared/inversify';
3-
import { Board, BoardsService, Port, Status } from '../common/protocol';
3+
import {
4+
AlreadyConnectedError,
5+
Board,
6+
BoardsService,
7+
Port,
8+
} from '../common/protocol';
49
import { CoreClientAware } from './core-client-provider';
510
import { MonitorService } from './monitor-service';
611
import { MonitorServiceFactory } from './monitor-service-factory';
@@ -36,7 +41,7 @@ export class MonitorManager extends CoreClientAware {
3641
private monitorServiceStartQueue: {
3742
monitorID: string;
3843
serviceStartParams: [Board, Port];
39-
connectToClient: (status: Status) => void;
44+
connectToClient: () => Promise<void>;
4045
}[] = [];
4146

4247
@inject(MonitorServiceFactory)
@@ -104,7 +109,7 @@ export class MonitorManager extends CoreClientAware {
104109
async startMonitor(
105110
board: Board,
106111
port: Port,
107-
connectToClient: (status: Status) => void
112+
connectToClient: () => Promise<void>
108113
): Promise<void> {
109114
const monitorID = this.monitorID(board.fqbn, port);
110115

@@ -127,8 +132,14 @@ export class MonitorManager extends CoreClientAware {
127132
return;
128133
}
129134

130-
const result = await monitor.start();
131-
connectToClient(result);
135+
try {
136+
await monitor.start();
137+
} catch (err) {
138+
if (!AlreadyConnectedError.is(err)) {
139+
throw err;
140+
}
141+
}
142+
await connectToClient();
132143
}
133144

134145
/**
@@ -202,8 +213,7 @@ export class MonitorManager extends CoreClientAware {
202213
async notifyUploadFinished(
203214
fqbn?: string | undefined,
204215
port?: Port
205-
): Promise<Status> {
206-
let status: Status = Status.NOT_CONNECTED;
216+
): Promise<void> {
207217
let portDidChangeOnUpload = false;
208218

209219
// We have no way of knowing which monitor
@@ -214,7 +224,7 @@ export class MonitorManager extends CoreClientAware {
214224

215225
const monitor = this.monitorServices.get(monitorID);
216226
if (monitor) {
217-
status = await monitor.start();
227+
await monitor.start();
218228
}
219229

220230
// this monitorID will only be present in "disposedForUpload"
@@ -232,7 +242,6 @@ export class MonitorManager extends CoreClientAware {
232242
}
233243

234244
await this.startQueuedServices(portDidChangeOnUpload);
235-
return status;
236245
}
237246

238247
async startQueuedServices(portDidChangeOnUpload: boolean): Promise<void> {
@@ -261,8 +270,8 @@ export class MonitorManager extends CoreClientAware {
261270
const monitorService = this.monitorServices.get(monitorID);
262271

263272
if (monitorService) {
264-
const result = await monitorService.start();
265-
connectToClient(result);
273+
await monitorService.start();
274+
await connectToClient();
266275
}
267276
}
268277
}

0 commit comments

Comments
 (0)