Skip to content

Commit 9cced4b

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 9cced4b

File tree

7 files changed

+272
-135
lines changed

7 files changed

+272
-135
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/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: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
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 { Board, BoardsService, Port } from '../common/protocol';
44
import { CoreClientAware } from './core-client-provider';
55
import { MonitorService } from './monitor-service';
66
import { MonitorServiceFactory } from './monitor-service-factory';
@@ -36,7 +36,7 @@ export class MonitorManager extends CoreClientAware {
3636
private monitorServiceStartQueue: {
3737
monitorID: string;
3838
serviceStartParams: [Board, Port];
39-
connectToClient: (status: Status) => void;
39+
connectToClient: () => Promise<void>;
4040
}[] = [];
4141

4242
@inject(MonitorServiceFactory)
@@ -104,7 +104,7 @@ export class MonitorManager extends CoreClientAware {
104104
async startMonitor(
105105
board: Board,
106106
port: Port,
107-
connectToClient: (status: Status) => void
107+
connectToClient: () => Promise<void>
108108
): Promise<void> {
109109
const monitorID = this.monitorID(board.fqbn, port);
110110

@@ -127,8 +127,8 @@ export class MonitorManager extends CoreClientAware {
127127
return;
128128
}
129129

130-
const result = await monitor.start();
131-
connectToClient(result);
130+
await monitor.start();
131+
await connectToClient();
132132
}
133133

134134
/**
@@ -202,8 +202,7 @@ export class MonitorManager extends CoreClientAware {
202202
async notifyUploadFinished(
203203
fqbn?: string | undefined,
204204
port?: Port
205-
): Promise<Status> {
206-
let status: Status = Status.NOT_CONNECTED;
205+
): Promise<void> {
207206
let portDidChangeOnUpload = false;
208207

209208
// We have no way of knowing which monitor
@@ -214,7 +213,7 @@ export class MonitorManager extends CoreClientAware {
214213

215214
const monitor = this.monitorServices.get(monitorID);
216215
if (monitor) {
217-
status = await monitor.start();
216+
await monitor.start();
218217
}
219218

220219
// this monitorID will only be present in "disposedForUpload"
@@ -232,7 +231,6 @@ export class MonitorManager extends CoreClientAware {
232231
}
233232

234233
await this.startQueuedServices(portDidChangeOnUpload);
235-
return status;
236234
}
237235

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

263261
if (monitorService) {
264-
const result = await monitorService.start();
265-
connectToClient(result);
262+
await monitorService.start();
263+
await connectToClient();
266264
}
267265
}
268266
}

0 commit comments

Comments
 (0)