Skip to content

Commit 6984c52

Browse files
authored
fix: Handle gracefully when trying to detect invalid sketch name error and folder is missing on filesystem (#1616)
- feat: generalized Node.js error handling - Gracefully handle when the sketch folder has been deleted - feat: spare detecting invalid sketch name error - The invalid sketch name detection requires at least one extra FS access. Do not try to detect the invalid sketch name error, but use the original `NotFound` from the CLI. - fix: typo Closes #1596 Signed-off-by: Akos Kitta <[email protected]>
1 parent 3a70547 commit 6984c52

File tree

6 files changed

+78
-28
lines changed

6 files changed

+78
-28
lines changed

Diff for: arduino-ide-extension/src/electron-main/theia/electron-main-application.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
SHOW_PLOTTER_WINDOW,
2929
} from '../../common/ipc-communication';
3030
import isValidPath = require('is-valid-path');
31+
import { ErrnoException } from '../../node/utils/errors';
3132

3233
app.commandLine.appendSwitch('disable-http-cache');
3334

@@ -172,7 +173,7 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
172173
try {
173174
stats = await fs.stat(path);
174175
} catch (err) {
175-
if ('code' in err && err.code === 'ENOENT') {
176+
if (ErrnoException.isENOENT(err)) {
176177
return undefined;
177178
}
178179
throw err;
@@ -215,7 +216,7 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
215216
const resolved = await fs.realpath(resolve(cwd, maybePath));
216217
return resolved;
217218
} catch (err) {
218-
if ('code' in err && err.code === 'ENOENT') {
219+
if (ErrnoException.isENOENT(err)) {
219220
return undefined;
220221
}
221222
throw err;

Diff for: arduino-ide-extension/src/node/arduino-daemon-impl.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { BackendApplicationContribution } from '@theia/core/lib/node/backend-app
1616
import { ArduinoDaemon, NotificationServiceServer } from '../common/protocol';
1717
import { CLI_CONFIG } from './cli-config';
1818
import { getExecPath, spawnCommand } from './exec-util';
19+
import { ErrnoException } from './utils/errors';
1920

2021
@injectable()
2122
export class ArduinoDaemonImpl
@@ -184,7 +185,7 @@ export class ArduinoDaemonImpl
184185
}
185186
return false;
186187
} catch (error) {
187-
if ('code' in error && error.code === 'ENOENT') {
188+
if (ErrnoException.isENOENT(error)) {
188189
return false;
189190
}
190191
throw error;

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { DefaultCliConfig, CLI_CONFIG } from './cli-config';
2626
import { Deferred } from '@theia/core/lib/common/promise-util';
2727
import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
2828
import { deepClone } from '@theia/core';
29+
import { ErrnoException } from './utils/errors';
2930

3031
const deepmerge = require('deepmerge');
3132

@@ -146,7 +147,7 @@ export class ConfigServiceImpl
146147
const fallbackModel = await this.getFallbackCliConfig();
147148
return deepmerge(fallbackModel, model) as DefaultCliConfig;
148149
} catch (error) {
149-
if ('code' in error && error.code === 'ENOENT') {
150+
if (ErrnoException.isENOENT(error)) {
150151
if (initializeIfAbsent) {
151152
await this.initCliConfigTo(dirname(cliConfigPath));
152153
return this.loadCliConfig(false);

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

+36-23
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
TempSketchPrefix,
3434
} from './is-temp-sketch';
3535
import { join } from 'path';
36+
import { ErrnoException } from './utils/errors';
3637

3738
const RecentSketches = 'recent-sketches.json';
3839
const DefaultIno = `void setup() {
@@ -187,6 +188,13 @@ export class SketchesServiceImpl
187188
}
188189

189190
async loadSketch(uri: string): Promise<SketchWithDetails> {
191+
return this.doLoadSketch(uri);
192+
}
193+
194+
private async doLoadSketch(
195+
uri: string,
196+
detectInvalidSketchNameError = true
197+
): Promise<SketchWithDetails> {
190198
const { client, instance } = await this.coreClient;
191199
const req = new LoadSketchRequest();
192200
const requestSketchPath = FileUri.fsPath(uri);
@@ -201,17 +209,19 @@ export class SketchesServiceImpl
201209
if (err) {
202210
let rejectWith: unknown = err;
203211
if (isNotFoundError(err)) {
204-
const invalidMainSketchFilePath = await isInvalidSketchNameError(
205-
err,
206-
requestSketchPath
207-
);
208-
if (invalidMainSketchFilePath) {
209-
rejectWith = SketchesError.InvalidName(
210-
err.details,
211-
FileUri.create(invalidMainSketchFilePath).toString()
212+
rejectWith = SketchesError.NotFound(err.details, uri);
213+
// detecting the invalid sketch name error is not for free as it requires multiple filesystem access.
214+
if (detectInvalidSketchNameError) {
215+
const invalidMainSketchFilePath = await isInvalidSketchNameError(
216+
err,
217+
requestSketchPath
212218
);
213-
} else {
214-
rejectWith = SketchesError.NotFound(err.details, uri);
219+
if (invalidMainSketchFilePath) {
220+
rejectWith = SketchesError.InvalidName(
221+
err.details,
222+
FileUri.create(invalidMainSketchFilePath).toString()
223+
);
224+
}
215225
}
216226
}
217227
reject(rejectWith);
@@ -278,7 +288,7 @@ export class SketchesServiceImpl
278288
);
279289
}
280290
} catch (err) {
281-
if ('code' in err && err.code === 'ENOENT') {
291+
if (ErrnoException.isENOENT(err)) {
282292
this.logger.debug(
283293
`<<< '${RecentSketches}' does not exist yet. This is normal behavior. Falling back to empty data.`
284294
);
@@ -317,7 +327,7 @@ export class SketchesServiceImpl
317327

318328
let sketch: Sketch | undefined = undefined;
319329
try {
320-
sketch = await this.loadSketch(uri);
330+
sketch = await this.doLoadSketch(uri, false);
321331
this.logger.debug(
322332
`Loaded sketch ${JSON.stringify(
323333
sketch
@@ -390,7 +400,7 @@ export class SketchesServiceImpl
390400
)) {
391401
let sketch: SketchWithDetails | undefined = undefined;
392402
try {
393-
sketch = await this.loadSketch(uri);
403+
sketch = await this.doLoadSketch(uri, false);
394404
} catch {}
395405
if (!sketch) {
396406
needsUpdate = true;
@@ -413,14 +423,14 @@ export class SketchesServiceImpl
413423

414424
async cloneExample(uri: string): Promise<Sketch> {
415425
const [sketch, parentPath] = await Promise.all([
416-
this.loadSketch(uri),
426+
this.doLoadSketch(uri, false),
417427
this.createTempFolder(),
418428
]);
419429
const destinationUri = FileUri.create(
420430
path.join(parentPath, sketch.name)
421431
).toString();
422432
const copiedSketchUri = await this.copy(sketch, { destinationUri });
423-
return this.loadSketch(copiedSketchUri);
433+
return this.doLoadSketch(copiedSketchUri, false);
424434
}
425435

426436
async createNewSketch(): Promise<Sketch> {
@@ -477,7 +487,7 @@ export class SketchesServiceImpl
477487
fs.mkdir(sketchDir, { recursive: true }),
478488
]);
479489
await fs.writeFile(sketchFile, inoContent, { encoding: 'utf8' });
480-
return this.loadSketch(FileUri.create(sketchDir).toString());
490+
return this.doLoadSketch(FileUri.create(sketchDir).toString(), false);
481491
}
482492

483493
/**
@@ -528,7 +538,7 @@ export class SketchesServiceImpl
528538
uri: string
529539
): Promise<SketchWithDetails | undefined> {
530540
try {
531-
const sketch = await this.loadSketch(uri);
541+
const sketch = await this.doLoadSketch(uri, false);
532542
return sketch;
533543
} catch (err) {
534544
if (SketchesError.NotFound.is(err) || SketchesError.InvalidName.is(err)) {
@@ -553,7 +563,7 @@ export class SketchesServiceImpl
553563
}
554564
// Nothing to do when source and destination are the same.
555565
if (sketch.uri === destinationUri) {
556-
await this.loadSketch(sketch.uri); // Sanity check.
566+
await this.doLoadSketch(sketch.uri, false); // Sanity check.
557567
return sketch.uri;
558568
}
559569

@@ -574,7 +584,10 @@ export class SketchesServiceImpl
574584
if (oldPath !== newPath) {
575585
await fs.rename(oldPath, newPath);
576586
}
577-
await this.loadSketch(FileUri.create(destinationPath).toString()); // Sanity check.
587+
await this.doLoadSketch(
588+
FileUri.create(destinationPath).toString(),
589+
false
590+
); // Sanity check.
578591
resolve();
579592
} catch (e) {
580593
reject(e);
@@ -596,7 +609,7 @@ export class SketchesServiceImpl
596609
}
597610

598611
async archive(sketch: Sketch, destinationUri: string): Promise<string> {
599-
await this.loadSketch(sketch.uri); // sanity check
612+
await this.doLoadSketch(sketch.uri, false); // sanity check
600613
const { client } = await this.coreClient;
601614
const archivePath = FileUri.fsPath(destinationUri);
602615
// The CLI cannot override existing archives, so we have to wipe it manually: https://github.com/arduino/arduino-cli/issues/1160
@@ -666,7 +679,7 @@ export class SketchesServiceImpl
666679

667680
return this.tryParse(raw);
668681
} catch (err) {
669-
if ('code' in err && err.code === 'ENOENT') {
682+
if (ErrnoException.isENOENT(err)) {
670683
return undefined;
671684
}
672685
throw err;
@@ -695,7 +708,7 @@ export class SketchesServiceImpl
695708
});
696709
this.inoContent.resolve(inoContent);
697710
} catch (err) {
698-
if ('code' in err && err.code === 'ENOENT') {
711+
if (ErrnoException.isENOENT(err)) {
699712
// Ignored. The custom `.ino` blueprint file is optional.
700713
} else {
701714
throw err;
@@ -763,7 +776,7 @@ async function isInvalidSketchNameError(
763776
.map((name) => path.join(requestSketchPath, name))[0]
764777
);
765778
} catch (err) {
766-
if ('code' in err && err.code === 'ENOTDIR') {
779+
if (ErrnoException.isENOENT(err) || ErrnoException.isENOTDIR(err)) {
767780
return undefined;
768781
}
769782
throw err;

Diff for: arduino-ide-extension/src/node/utils/errors.ts

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
export type ErrnoException = Error & { code: string; errno: number };
2+
export namespace ErrnoException {
3+
export function is(arg: unknown): arg is ErrnoException {
4+
if (arg instanceof Error) {
5+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
6+
const error = arg as any;
7+
return (
8+
'code' in error &&
9+
'errno' in error &&
10+
typeof error['code'] === 'string' &&
11+
typeof error['errno'] === 'number'
12+
);
13+
}
14+
return false;
15+
}
16+
17+
/**
18+
* (No such file or directory): Commonly raised by `fs` operations to indicate that a component of the specified pathname does not exist — no entity (file or directory) could be found by the given path.
19+
*/
20+
export function isENOENT(
21+
arg: unknown
22+
): arg is ErrnoException & { code: 'ENOENT' } {
23+
return is(arg) && arg.code === 'ENOENT';
24+
}
25+
26+
/**
27+
* (Not a directory): A component of the given pathname existed, but was not a directory as expected. Commonly raised by `fs.readdir`.
28+
*/
29+
export function isENOTDIR(
30+
arg: unknown
31+
): arg is ErrnoException & { code: 'ENOTDIR' } {
32+
return is(arg) && arg.code === 'ENOTDIR';
33+
}
34+
}

Diff for: scripts/i18n/transifex-pull.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const requestTranslationDownload = async (relationships) => {
5454

5555
const getTranslationDownloadStatus = async (language, downloadRequestId) => {
5656
// The download request status must be asked from time to time, if it's
57-
// still pending we try again using exponentional backoff starting from 2.5 seconds.
57+
// still pending we try again using exponential backoff starting from 2.5 seconds.
5858
let backoffMs = 2500;
5959
while (true) {
6060
const url = transifex.url(

0 commit comments

Comments
 (0)