Skip to content

Commit a2a14af

Browse files
author
Akos Kitta
committed
fix: warn user when IDE cannot save the sketch
Happens when the IDE2 backend process crashes, and the communication drops between the client and the server. Closes #2081 Signed-off-by: Akos Kitta <[email protected]>
1 parent d1fa6d4 commit a2a14af

File tree

5 files changed

+71
-14
lines changed

5 files changed

+71
-14
lines changed

Diff for: arduino-ide-extension/src/browser/contributions/contribution.ts

+4
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import { MainMenuManager } from '../../common/main-menu-manager';
6868
import { ConfigServiceClient } from '../config/config-service-client';
6969
import { ApplicationShell } from '@theia/core/lib/browser/shell/application-shell';
7070
import { DialogService } from '../dialog-service';
71+
import { ApplicationConnectionStatusContribution } from '../theia/core/connection-status-service';
7172

7273
export {
7374
Command,
@@ -172,6 +173,9 @@ export abstract class SketchContribution extends Contribution {
172173
@inject(EnvVariablesServer)
173174
protected readonly envVariableServer: EnvVariablesServer;
174175

176+
@inject(ApplicationConnectionStatusContribution)
177+
protected readonly connectionStatusService: ApplicationConnectionStatusContribution;
178+
175179
protected async sourceOverride(): Promise<Record<string, string>> {
176180
const override: Record<string, string> = {};
177181
const sketch = await this.sketchServiceClient.currentSketch();

Diff for: arduino-ide-extension/src/browser/contributions/save-as-sketch.ts

+5
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
RenameCloudSketch,
2525
RenameCloudSketchParams,
2626
} from './rename-cloud-sketch';
27+
import { assertConnectedToBackend } from './save-sketch';
2728

2829
@injectable()
2930
export class SaveAsSketch extends CloudSketchContribution {
@@ -64,6 +65,10 @@ export class SaveAsSketch extends CloudSketchContribution {
6465
markAsRecentlyOpened,
6566
}: SaveAsSketch.Options = SaveAsSketch.Options.DEFAULT
6667
): Promise<boolean> {
68+
assertConnectedToBackend({
69+
connectionStatusService: this.connectionStatusService,
70+
messageService: this.messageService,
71+
});
6772
const sketch = await this.sketchServiceClient.currentSketch();
6873
if (!CurrentSketch.isValid(sketch)) {
6974
return false;

Diff for: arduino-ide-extension/src/browser/contributions/save-sketch.ts

+27-6
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
1-
import { injectable } from '@theia/core/shared/inversify';
21
import { CommonCommands } from '@theia/core/lib/browser/common-frontend-contribution';
2+
import { MessageService } from '@theia/core/lib/common/message-service';
3+
import { nls } from '@theia/core/lib/common/nls';
4+
import { injectable } from '@theia/core/shared/inversify';
35
import { ArduinoMenus } from '../menu/arduino-menus';
4-
import { SaveAsSketch } from './save-as-sketch';
6+
import { CurrentSketch } from '../sketches-service-client-impl';
7+
import { ApplicationConnectionStatusContribution } from '../theia/core/connection-status-service';
58
import {
6-
SketchContribution,
79
Command,
810
CommandRegistry,
9-
MenuModelRegistry,
1011
KeybindingRegistry,
12+
MenuModelRegistry,
13+
SketchContribution,
1114
} from './contribution';
12-
import { nls } from '@theia/core/lib/common';
13-
import { CurrentSketch } from '../sketches-service-client-impl';
15+
import { SaveAsSketch } from './save-as-sketch';
1416

1517
@injectable()
1618
export class SaveSketch extends SketchContribution {
@@ -36,6 +38,10 @@ export class SaveSketch extends SketchContribution {
3638
}
3739

3840
async saveSketch(): Promise<void> {
41+
assertConnectedToBackend({
42+
connectionStatusService: this.connectionStatusService,
43+
messageService: this.messageService,
44+
});
3945
const sketch = await this.sketchServiceClient.currentSketch();
4046
if (!CurrentSketch.isValid(sketch)) {
4147
return;
@@ -63,3 +69,18 @@ export namespace SaveSketch {
6369
};
6470
}
6571
}
72+
73+
// https://github.com/arduino/arduino-ide/issues/2081
74+
export function assertConnectedToBackend(param: {
75+
connectionStatusService: ApplicationConnectionStatusContribution;
76+
messageService: MessageService;
77+
}): void {
78+
if (param.connectionStatusService.offlineStatus === 'backend') {
79+
const message = nls.localize(
80+
'theia/core/couldNotSave',
81+
'Could not save the sketch. Please copy your unsaved work into your favorite text editor, and restart the IDE.'
82+
);
83+
param.messageService.error(message);
84+
throw new Error(message);
85+
}
86+
}

Diff for: arduino-ide-extension/src/browser/theia/core/connection-status-service.ts

+15-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
FrontendConnectionStatusService as TheiaFrontendConnectionStatusService,
55
} from '@theia/core/lib/browser/connection-status-service';
66
import type { FrontendApplicationContribution } from '@theia/core/lib/browser/frontend-application';
7+
import { WebSocketConnectionProvider } from '@theia/core/lib/browser/index';
78
import { StatusBarAlignment } from '@theia/core/lib/browser/status-bar/status-bar';
89
import { Disposable } from '@theia/core/lib/common/disposable';
910
import { Emitter, Event } from '@theia/core/lib/common/event';
@@ -16,11 +17,11 @@ import {
1617
postConstruct,
1718
} from '@theia/core/shared/inversify';
1819
import { NotificationManager } from '@theia/messages/lib/browser/notifications-manager';
20+
import debounce from 'lodash.debounce';
1921
import { ArduinoDaemon } from '../../../common/protocol';
2022
import { assertUnreachable } from '../../../common/utils';
2123
import { CreateFeatures } from '../../create/create-features';
2224
import { NotificationCenter } from '../../notification-center';
23-
import debounce from 'lodash.debounce';
2425

2526
@injectable()
2627
export class IsOnline implements FrontendApplicationContribution {
@@ -113,6 +114,8 @@ export class FrontendConnectionStatusService extends TheiaFrontendConnectionStat
113114
private readonly daemonPort: DaemonPort;
114115
@inject(IsOnline)
115116
private readonly isOnline: IsOnline;
117+
@inject(WebSocketConnectionProvider)
118+
private readonly connectionProvider: WebSocketConnectionProvider;
116119

117120
@postConstruct()
118121
protected override async init(): Promise<void> {
@@ -125,6 +128,10 @@ export class FrontendConnectionStatusService extends TheiaFrontendConnectionStat
125128
}
126129

127130
protected override async performPingRequest(): Promise<void> {
131+
if (!this.connectionProvider['socket'].connected) {
132+
this.updateStatus(false);
133+
return;
134+
}
128135
try {
129136
await this.pingService.ping();
130137
this.updateStatus(this.isOnline.online);
@@ -164,6 +171,8 @@ export class ApplicationConnectionStatusContribution extends TheiaApplicationCon
164171
private readonly notificationManager: NotificationManager;
165172
@inject(CreateFeatures)
166173
private readonly createFeatures: CreateFeatures;
174+
@inject(WebSocketConnectionProvider)
175+
private readonly connectionProvider: WebSocketConnectionProvider;
167176

168177
private readonly offlineStatusDidChangeEmitter = new Emitter<
169178
OfflineConnectionStatus | undefined
@@ -190,9 +199,10 @@ export class ApplicationConnectionStatusContribution extends TheiaApplicationCon
190199
}
191200

192201
protected override handleOffline(): void {
193-
const params = {
202+
const params = <OfflineMessageParams>{
194203
port: this.daemonPort.port,
195204
online: this.isOnline.online,
205+
backendConnected: this.connectionProvider['socket'].connected, // https://github.com/arduino/arduino-ide/issues/2081
196206
};
197207
this._offlineStatus = offlineConnectionStatusType(params);
198208
const { text, tooltip } = offlineMessage(params);
@@ -248,6 +258,7 @@ export class ApplicationConnectionStatusContribution extends TheiaApplicationCon
248258
interface OfflineMessageParams {
249259
readonly port: string | undefined;
250260
readonly online: boolean;
261+
readonly backendConnected: boolean;
251262
}
252263
interface OfflineMessage {
253264
readonly text: string;
@@ -272,8 +283,8 @@ export function offlineMessage(params: OfflineMessageParams): OfflineMessage {
272283
function offlineConnectionStatusType(
273284
params: OfflineMessageParams
274285
): OfflineConnectionStatus {
275-
const { port, online } = params;
276-
if (port && online) {
286+
const { port, online, backendConnected } = params;
287+
if (!backendConnected || (port && online)) {
277288
return 'backend';
278289
}
279290
if (!port) {

Diff for: arduino-ide-extension/src/test/browser/connection-status-service.test.ts

+20-4
Original file line numberDiff line numberDiff line change
@@ -20,25 +20,41 @@ disableJSDOM();
2020
describe('connection-status-service', () => {
2121
describe('offlineMessage', () => {
2222
it('should warn about the offline backend if connected to both CLI daemon and Internet but offline', () => {
23-
const actual = offlineMessage({ port: '50051', online: true });
23+
const actual = offlineMessage({
24+
port: '50051',
25+
online: true,
26+
backendConnected: false,
27+
});
2428
expect(actual.text).to.be.equal(backendOfflineText);
2529
expect(actual.tooltip).to.be.equal(backendOfflineTooltip);
2630
});
2731

2832
it('should warn about the offline CLI daemon if the CLI daemon port is missing but has Internet connection', () => {
29-
const actual = offlineMessage({ port: undefined, online: true });
33+
const actual = offlineMessage({
34+
port: undefined,
35+
online: true,
36+
backendConnected: true,
37+
});
3038
expect(actual.text.endsWith(daemonOfflineText)).to.be.true;
3139
expect(actual.tooltip).to.be.equal(daemonOfflineTooltip);
3240
});
3341

3442
it('should warn about the offline CLI daemon if the CLI daemon port is missing and has no Internet connection', () => {
35-
const actual = offlineMessage({ port: undefined, online: false });
43+
const actual = offlineMessage({
44+
port: undefined,
45+
online: false,
46+
backendConnected: true,
47+
});
3648
expect(actual.text.endsWith(daemonOfflineText)).to.be.true;
3749
expect(actual.tooltip).to.be.equal(daemonOfflineTooltip);
3850
});
3951

4052
it('should warn about no Internet connection if CLI daemon port is available but the Internet connection is offline', () => {
41-
const actual = offlineMessage({ port: '50051', online: false });
53+
const actual = offlineMessage({
54+
port: '50051',
55+
online: false,
56+
backendConnected: true,
57+
});
4258
expect(actual.text.endsWith(offlineText)).to.be.true;
4359
expect(actual.tooltip).to.be.equal(offlineTooltip);
4460
});

0 commit comments

Comments
 (0)