Skip to content

Commit

Permalink
fix: warn user when IDE cannot save the sketch
Browse files Browse the repository at this point in the history
Happens when the IDE2 backend process crashes, and the communication
drops between the client and the server.

Closes #2081

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
  • Loading branch information
Akos Kitta committed May 30, 2023
1 parent d1843ff commit b05c7ca
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import { MainMenuManager } from '../../common/main-menu-manager';
import { ConfigServiceClient } from '../config/config-service-client';
import { ApplicationShell } from '@theia/core/lib/browser/shell/application-shell';
import { DialogService } from '../dialog-service';
import { ApplicationConnectionStatusContribution } from '../theia/core/connection-status-service';

export {
Command,
Expand Down Expand Up @@ -172,6 +173,9 @@ export abstract class SketchContribution extends Contribution {
@inject(EnvVariablesServer)
protected readonly envVariableServer: EnvVariablesServer;

@inject(ApplicationConnectionStatusContribution)
protected readonly connectionStatusService: ApplicationConnectionStatusContribution;

protected async sourceOverride(): Promise<Record<string, string>> {
const override: Record<string, string> = {};
const sketch = await this.sketchServiceClient.currentSketch();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
RenameCloudSketch,
RenameCloudSketchParams,
} from './rename-cloud-sketch';
import { assertConnectedToBackend } from './save-sketch';

@injectable()
export class SaveAsSketch extends CloudSketchContribution {
Expand Down Expand Up @@ -64,6 +65,10 @@ export class SaveAsSketch extends CloudSketchContribution {
markAsRecentlyOpened,
}: SaveAsSketch.Options = SaveAsSketch.Options.DEFAULT
): Promise<boolean> {
assertConnectedToBackend({
connectionStatusService: this.connectionStatusService,
messageService: this.messageService,
});
const sketch = await this.sketchServiceClient.currentSketch();
if (!CurrentSketch.isValid(sketch)) {
return false;
Expand Down
33 changes: 27 additions & 6 deletions arduino-ide-extension/src/browser/contributions/save-sketch.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import { injectable } from '@theia/core/shared/inversify';
import { CommonCommands } from '@theia/core/lib/browser/common-frontend-contribution';
import { MessageService } from '@theia/core/lib/common/message-service';
import { nls } from '@theia/core/lib/common/nls';
import { injectable } from '@theia/core/shared/inversify';
import { ArduinoMenus } from '../menu/arduino-menus';
import { SaveAsSketch } from './save-as-sketch';
import { CurrentSketch } from '../sketches-service-client-impl';
import { ApplicationConnectionStatusContribution } from '../theia/core/connection-status-service';
import {
SketchContribution,
Command,
CommandRegistry,
MenuModelRegistry,
KeybindingRegistry,
MenuModelRegistry,
SketchContribution,
} from './contribution';
import { nls } from '@theia/core/lib/common';
import { CurrentSketch } from '../sketches-service-client-impl';
import { SaveAsSketch } from './save-as-sketch';

@injectable()
export class SaveSketch extends SketchContribution {
Expand All @@ -36,6 +38,10 @@ export class SaveSketch extends SketchContribution {
}

async saveSketch(): Promise<void> {
assertConnectedToBackend({
connectionStatusService: this.connectionStatusService,
messageService: this.messageService,
});
const sketch = await this.sketchServiceClient.currentSketch();
if (!CurrentSketch.isValid(sketch)) {
return;
Expand Down Expand Up @@ -63,3 +69,18 @@ export namespace SaveSketch {
};
}
}

// https://github.com/arduino/arduino-ide/issues/2081
export function assertConnectedToBackend(param: {
connectionStatusService: ApplicationConnectionStatusContribution;
messageService: MessageService;
}): void {
if (param.connectionStatusService.offlineStatus === 'backend') {
const message = nls.localize(
'theia/core/couldNotSave',
'Could not save the sketch. Please copy your unsaved work into your favorite text editor, and restart the IDE.'
);
param.messageService.error(message);
throw new Error(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
FrontendConnectionStatusService as TheiaFrontendConnectionStatusService,
} from '@theia/core/lib/browser/connection-status-service';
import type { FrontendApplicationContribution } from '@theia/core/lib/browser/frontend-application';
import { WebSocketConnectionProvider } from '@theia/core/lib/browser/index';
import { StatusBarAlignment } from '@theia/core/lib/browser/status-bar/status-bar';
import { Disposable } from '@theia/core/lib/common/disposable';
import { Emitter, Event } from '@theia/core/lib/common/event';
Expand All @@ -16,11 +17,11 @@ import {
postConstruct,
} from '@theia/core/shared/inversify';
import { NotificationManager } from '@theia/messages/lib/browser/notifications-manager';
import debounce from 'lodash.debounce';
import { ArduinoDaemon } from '../../../common/protocol';
import { assertUnreachable } from '../../../common/utils';
import { CreateFeatures } from '../../create/create-features';
import { NotificationCenter } from '../../notification-center';
import debounce from 'lodash.debounce';

@injectable()
export class IsOnline implements FrontendApplicationContribution {
Expand Down Expand Up @@ -113,6 +114,8 @@ export class FrontendConnectionStatusService extends TheiaFrontendConnectionStat
private readonly daemonPort: DaemonPort;
@inject(IsOnline)
private readonly isOnline: IsOnline;
@inject(WebSocketConnectionProvider)
private readonly connectionProvider: WebSocketConnectionProvider;

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

protected override async performPingRequest(): Promise<void> {
if (!this.connectionProvider['socket'].connected) {
this.updateStatus(false);
return;
}
try {
await this.pingService.ping();
this.updateStatus(this.isOnline.online);
Expand Down Expand Up @@ -164,6 +171,8 @@ export class ApplicationConnectionStatusContribution extends TheiaApplicationCon
private readonly notificationManager: NotificationManager;
@inject(CreateFeatures)
private readonly createFeatures: CreateFeatures;
@inject(WebSocketConnectionProvider)
private readonly connectionProvider: WebSocketConnectionProvider;

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

protected override handleOffline(): void {
const params = {
const params = <OfflineMessageParams>{
port: this.daemonPort.port,
online: this.isOnline.online,
backendConnected: this.connectionProvider['socket'].connected, // https://github.com/arduino/arduino-ide/issues/2081
};
this._offlineStatus = offlineConnectionStatusType(params);
const { text, tooltip } = offlineMessage(params);
Expand Down Expand Up @@ -248,6 +258,7 @@ export class ApplicationConnectionStatusContribution extends TheiaApplicationCon
interface OfflineMessageParams {
readonly port: string | undefined;
readonly online: boolean;
readonly backendConnected: boolean;
}
interface OfflineMessage {
readonly text: string;
Expand All @@ -272,8 +283,8 @@ export function offlineMessage(params: OfflineMessageParams): OfflineMessage {
function offlineConnectionStatusType(
params: OfflineMessageParams
): OfflineConnectionStatus {
const { port, online } = params;
if (port && online) {
const { port, online, backendConnected } = params;
if (!backendConnected || (port && online)) {
return 'backend';
}
if (!port) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,41 @@ disableJSDOM();
describe('connection-status-service', () => {
describe('offlineMessage', () => {
it('should warn about the offline backend if connected to both CLI daemon and Internet but offline', () => {
const actual = offlineMessage({ port: '50051', online: true });
const actual = offlineMessage({
port: '50051',
online: true,
backendConnected: false,
});
expect(actual.text).to.be.equal(backendOfflineText);
expect(actual.tooltip).to.be.equal(backendOfflineTooltip);
});

it('should warn about the offline CLI daemon if the CLI daemon port is missing but has Internet connection', () => {
const actual = offlineMessage({ port: undefined, online: true });
const actual = offlineMessage({
port: undefined,
online: true,
backendConnected: true,
});
expect(actual.text.endsWith(daemonOfflineText)).to.be.true;
expect(actual.tooltip).to.be.equal(daemonOfflineTooltip);
});

it('should warn about the offline CLI daemon if the CLI daemon port is missing and has no Internet connection', () => {
const actual = offlineMessage({ port: undefined, online: false });
const actual = offlineMessage({
port: undefined,
online: false,
backendConnected: true,
});
expect(actual.text.endsWith(daemonOfflineText)).to.be.true;
expect(actual.tooltip).to.be.equal(daemonOfflineTooltip);
});

it('should warn about no Internet connection if CLI daemon port is available but the Internet connection is offline', () => {
const actual = offlineMessage({ port: '50051', online: false });
const actual = offlineMessage({
port: '50051',
online: false,
backendConnected: true,
});
expect(actual.text.endsWith(offlineText)).to.be.true;
expect(actual.tooltip).to.be.equal(offlineTooltip);
});
Expand Down

0 comments on commit b05c7ca

Please sign in to comment.