Skip to content

Commit 4e315ab

Browse files
author
Akos Kitta
committed
fix: propagate monitor errors to the frontend
Closes #1508 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
1 parent 9b49712 commit 4e315ab

File tree

8 files changed

+273
-134
lines changed

8 files changed

+273
-134
lines changed

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

Lines changed: 8 additions & 1 deletion
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> {

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

Lines changed: 1 addition & 1 deletion
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>

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

Lines changed: 155 additions & 21 deletions
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,158 @@ 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+
UploadInProgress: 6005,
81+
} as const;
82+
83+
export const ConnectionFailedError = createMonitorError(
84+
MonitorErrorCodes.ConnectionFailed
85+
);
86+
export const NotConnectedError = createMonitorError(
87+
MonitorErrorCodes.NotConnected
88+
);
89+
export const AlreadyConnectedError = createMonitorError(
90+
MonitorErrorCodes.AlreadyConnected
91+
);
92+
export const MissingConfigurationError = createMonitorError(
93+
MonitorErrorCodes.MissingConfiguration
94+
);
95+
export const UploadInProgressError = createMonitorError(
96+
MonitorErrorCodes.UploadInProgress
97+
);
98+
99+
export function createConnectionFailedError(
100+
port: Port,
101+
details?: string
102+
): ApplicationError<number, PortDescriptor> {
103+
const { protocol, protocolLabel, address, addressLabel } = port;
104+
return ConnectionFailedError(
105+
connectionFailedErrorLabel(addressLabel, protocolLabel, details),
106+
{ protocol, address }
107+
);
79108
}
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-
};
109+
110+
export function createNotConnectedError(
111+
port: Port
112+
): ApplicationError<number, PortDescriptor> {
113+
const { protocol, protocolLabel, address, addressLabel } = port;
114+
return NotConnectedError(
115+
notConnectedErrorLabel(addressLabel, protocolLabel),
116+
{ protocol, address }
117+
);
118+
}
119+
120+
export function createAlreadyConnectedError(
121+
port: Port
122+
): ApplicationError<number, PortDescriptor> {
123+
const { protocol, protocolLabel, address, addressLabel } = port;
124+
return AlreadyConnectedError(
125+
alreadyConnectedErrorLabel(addressLabel, protocolLabel),
126+
{ protocol, address }
127+
);
128+
}
129+
130+
export function createMissingConfigurationError(
131+
port: Port
132+
): ApplicationError<number, PortDescriptor> {
133+
const { protocol, protocolLabel, address, addressLabel } = port;
134+
return MissingConfigurationError(
135+
missingConfigurationErrorLabel(addressLabel, protocolLabel),
136+
{ protocol, address }
137+
);
138+
}
139+
140+
export function createUploadInProgressError(
141+
port: Port
142+
): ApplicationError<number, PortDescriptor> {
143+
const { protocol, protocolLabel, address, addressLabel } = port;
144+
return UploadInProgressError(
145+
uploadInProgressErrorLabel(addressLabel, protocolLabel),
146+
{ protocol, address }
147+
);
148+
}
149+
150+
/**
151+
* Bare minimum representation of a port. Supports neither UI labels nor properties.
152+
*/
153+
export interface PortDescriptor {
154+
readonly protocol: string;
155+
readonly address: string;
156+
}
157+
158+
export function createPortDescriptor(port: Port): PortDescriptor {
159+
const { protocol, address } = port;
160+
return { protocol, address };
161+
}
162+
163+
function createMonitorError(
164+
code: number
165+
): ApplicationError.Constructor<number, PortDescriptor> {
166+
return ApplicationError.declare(
167+
code,
168+
(message: string, data: PortDescriptor) => ({ data, message })
169+
);
170+
}
171+
172+
export function connectionFailedErrorLabel(
173+
addressLabel: string,
174+
protocolLabel: string,
175+
details?: string
176+
): string {
177+
const formattedDetails = details ? `: ${details}.` : '.';
178+
return nls.localize(
179+
'arduino/monitor/connectionFailedError',
180+
'Could not connect to {0} {1} port{2}',
181+
addressLabel,
182+
protocolLabel,
183+
formattedDetails
184+
);
185+
}
186+
export function notConnectedErrorLabel(
187+
addressLabel: string,
188+
protocolLabel: string
189+
): string {
190+
return nls.localize(
191+
'arduino/monitor/notConnectedError',
192+
'Not connected to {0} {1} port.',
193+
addressLabel,
194+
protocolLabel
195+
);
196+
}
197+
export function missingConfigurationErrorLabel(
198+
addressLabel: string,
199+
protocolLabel: string
200+
): string {
201+
return nls.localize(
202+
'arduino/monitor/missingConfigurationError',
203+
'Could not connect to {0} {1} port. The monitor configuration is missing.',
204+
addressLabel,
205+
protocolLabel
206+
);
207+
}
208+
export function uploadInProgressErrorLabel(
209+
addressLabel: string,
210+
protocolLabel: string
211+
): string {
212+
return nls.localize(
213+
'arduino/monitor/uploadInProgressError',
214+
'Could not connect to {0} {1} port. An upload is in progress.',
215+
addressLabel,
216+
protocolLabel
217+
);
218+
}
219+
export function alreadyConnectedErrorLabel(
220+
addressLabel: string,
221+
protocolLabel: string
222+
): string {
223+
return nls.localize(
224+
'arduino/monitor/alreadyConnectedError',
225+
'Could not connect to {0} {1} port. Already connected.',
226+
addressLabel,
227+
protocolLabel
228+
);
95229
}

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

Lines changed: 2 additions & 2 deletions
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
}

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

Lines changed: 2 additions & 6 deletions
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
}

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

Lines changed: 20 additions & 11 deletions
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)