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 <a.kitta@arduino.cc>
This commit is contained in:
Akos Kitta 2022-11-10 11:11:35 +01:00 committed by GitHub
parent 3a70547770
commit 6984c52b92
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 78 additions and 28 deletions

View File

@ -28,6 +28,7 @@ import {
SHOW_PLOTTER_WINDOW, SHOW_PLOTTER_WINDOW,
} from '../../common/ipc-communication'; } from '../../common/ipc-communication';
import isValidPath = require('is-valid-path'); import isValidPath = require('is-valid-path');
import { ErrnoException } from '../../node/utils/errors';
app.commandLine.appendSwitch('disable-http-cache'); app.commandLine.appendSwitch('disable-http-cache');
@ -172,7 +173,7 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
try { try {
stats = await fs.stat(path); stats = await fs.stat(path);
} catch (err) { } catch (err) {
if ('code' in err && err.code === 'ENOENT') { if (ErrnoException.isENOENT(err)) {
return undefined; return undefined;
} }
throw err; throw err;
@ -215,7 +216,7 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
const resolved = await fs.realpath(resolve(cwd, maybePath)); const resolved = await fs.realpath(resolve(cwd, maybePath));
return resolved; return resolved;
} catch (err) { } catch (err) {
if ('code' in err && err.code === 'ENOENT') { if (ErrnoException.isENOENT(err)) {
return undefined; return undefined;
} }
throw err; throw err;

View File

@ -16,6 +16,7 @@ import { BackendApplicationContribution } from '@theia/core/lib/node/backend-app
import { ArduinoDaemon, NotificationServiceServer } from '../common/protocol'; import { ArduinoDaemon, NotificationServiceServer } from '../common/protocol';
import { CLI_CONFIG } from './cli-config'; import { CLI_CONFIG } from './cli-config';
import { getExecPath, spawnCommand } from './exec-util'; import { getExecPath, spawnCommand } from './exec-util';
import { ErrnoException } from './utils/errors';
@injectable() @injectable()
export class ArduinoDaemonImpl export class ArduinoDaemonImpl
@ -184,7 +185,7 @@ export class ArduinoDaemonImpl
} }
return false; return false;
} catch (error) { } catch (error) {
if ('code' in error && error.code === 'ENOENT') { if (ErrnoException.isENOENT(error)) {
return false; return false;
} }
throw error; throw error;

View File

@ -26,6 +26,7 @@ import { DefaultCliConfig, CLI_CONFIG } from './cli-config';
import { Deferred } from '@theia/core/lib/common/promise-util'; import { Deferred } from '@theia/core/lib/common/promise-util';
import { EnvVariablesServer } from '@theia/core/lib/common/env-variables'; import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
import { deepClone } from '@theia/core'; import { deepClone } from '@theia/core';
import { ErrnoException } from './utils/errors';
const deepmerge = require('deepmerge'); const deepmerge = require('deepmerge');
@ -146,7 +147,7 @@ export class ConfigServiceImpl
const fallbackModel = await this.getFallbackCliConfig(); const fallbackModel = await this.getFallbackCliConfig();
return deepmerge(fallbackModel, model) as DefaultCliConfig; return deepmerge(fallbackModel, model) as DefaultCliConfig;
} catch (error) { } catch (error) {
if ('code' in error && error.code === 'ENOENT') { if (ErrnoException.isENOENT(error)) {
if (initializeIfAbsent) { if (initializeIfAbsent) {
await this.initCliConfigTo(dirname(cliConfigPath)); await this.initCliConfigTo(dirname(cliConfigPath));
return this.loadCliConfig(false); return this.loadCliConfig(false);

View File

@ -33,6 +33,7 @@ import {
TempSketchPrefix, TempSketchPrefix,
} from './is-temp-sketch'; } from './is-temp-sketch';
import { join } from 'path'; import { join } from 'path';
import { ErrnoException } from './utils/errors';
const RecentSketches = 'recent-sketches.json'; const RecentSketches = 'recent-sketches.json';
const DefaultIno = `void setup() { const DefaultIno = `void setup() {
@ -187,6 +188,13 @@ export class SketchesServiceImpl
} }
async loadSketch(uri: string): Promise<SketchWithDetails> { async loadSketch(uri: string): Promise<SketchWithDetails> {
return this.doLoadSketch(uri);
}
private async doLoadSketch(
uri: string,
detectInvalidSketchNameError = true
): Promise<SketchWithDetails> {
const { client, instance } = await this.coreClient; const { client, instance } = await this.coreClient;
const req = new LoadSketchRequest(); const req = new LoadSketchRequest();
const requestSketchPath = FileUri.fsPath(uri); const requestSketchPath = FileUri.fsPath(uri);
@ -201,17 +209,19 @@ export class SketchesServiceImpl
if (err) { if (err) {
let rejectWith: unknown = err; let rejectWith: unknown = err;
if (isNotFoundError(err)) { if (isNotFoundError(err)) {
const invalidMainSketchFilePath = await isInvalidSketchNameError( rejectWith = SketchesError.NotFound(err.details, uri);
err, // detecting the invalid sketch name error is not for free as it requires multiple filesystem access.
requestSketchPath if (detectInvalidSketchNameError) {
); const invalidMainSketchFilePath = await isInvalidSketchNameError(
if (invalidMainSketchFilePath) { err,
rejectWith = SketchesError.InvalidName( requestSketchPath
err.details,
FileUri.create(invalidMainSketchFilePath).toString()
); );
} else { if (invalidMainSketchFilePath) {
rejectWith = SketchesError.NotFound(err.details, uri); rejectWith = SketchesError.InvalidName(
err.details,
FileUri.create(invalidMainSketchFilePath).toString()
);
}
} }
} }
reject(rejectWith); reject(rejectWith);
@ -278,7 +288,7 @@ export class SketchesServiceImpl
); );
} }
} catch (err) { } catch (err) {
if ('code' in err && err.code === 'ENOENT') { if (ErrnoException.isENOENT(err)) {
this.logger.debug( this.logger.debug(
`<<< '${RecentSketches}' does not exist yet. This is normal behavior. Falling back to empty data.` `<<< '${RecentSketches}' does not exist yet. This is normal behavior. Falling back to empty data.`
); );
@ -317,7 +327,7 @@ export class SketchesServiceImpl
let sketch: Sketch | undefined = undefined; let sketch: Sketch | undefined = undefined;
try { try {
sketch = await this.loadSketch(uri); sketch = await this.doLoadSketch(uri, false);
this.logger.debug( this.logger.debug(
`Loaded sketch ${JSON.stringify( `Loaded sketch ${JSON.stringify(
sketch sketch
@ -390,7 +400,7 @@ export class SketchesServiceImpl
)) { )) {
let sketch: SketchWithDetails | undefined = undefined; let sketch: SketchWithDetails | undefined = undefined;
try { try {
sketch = await this.loadSketch(uri); sketch = await this.doLoadSketch(uri, false);
} catch {} } catch {}
if (!sketch) { if (!sketch) {
needsUpdate = true; needsUpdate = true;
@ -413,14 +423,14 @@ export class SketchesServiceImpl
async cloneExample(uri: string): Promise<Sketch> { async cloneExample(uri: string): Promise<Sketch> {
const [sketch, parentPath] = await Promise.all([ const [sketch, parentPath] = await Promise.all([
this.loadSketch(uri), this.doLoadSketch(uri, false),
this.createTempFolder(), this.createTempFolder(),
]); ]);
const destinationUri = FileUri.create( const destinationUri = FileUri.create(
path.join(parentPath, sketch.name) path.join(parentPath, sketch.name)
).toString(); ).toString();
const copiedSketchUri = await this.copy(sketch, { destinationUri }); const copiedSketchUri = await this.copy(sketch, { destinationUri });
return this.loadSketch(copiedSketchUri); return this.doLoadSketch(copiedSketchUri, false);
} }
async createNewSketch(): Promise<Sketch> { async createNewSketch(): Promise<Sketch> {
@ -477,7 +487,7 @@ export class SketchesServiceImpl
fs.mkdir(sketchDir, { recursive: true }), fs.mkdir(sketchDir, { recursive: true }),
]); ]);
await fs.writeFile(sketchFile, inoContent, { encoding: 'utf8' }); await fs.writeFile(sketchFile, inoContent, { encoding: 'utf8' });
return this.loadSketch(FileUri.create(sketchDir).toString()); return this.doLoadSketch(FileUri.create(sketchDir).toString(), false);
} }
/** /**
@ -528,7 +538,7 @@ export class SketchesServiceImpl
uri: string uri: string
): Promise<SketchWithDetails | undefined> { ): Promise<SketchWithDetails | undefined> {
try { try {
const sketch = await this.loadSketch(uri); const sketch = await this.doLoadSketch(uri, false);
return sketch; return sketch;
} catch (err) { } catch (err) {
if (SketchesError.NotFound.is(err) || SketchesError.InvalidName.is(err)) { if (SketchesError.NotFound.is(err) || SketchesError.InvalidName.is(err)) {
@ -553,7 +563,7 @@ export class SketchesServiceImpl
} }
// Nothing to do when source and destination are the same. // Nothing to do when source and destination are the same.
if (sketch.uri === destinationUri) { if (sketch.uri === destinationUri) {
await this.loadSketch(sketch.uri); // Sanity check. await this.doLoadSketch(sketch.uri, false); // Sanity check.
return sketch.uri; return sketch.uri;
} }
@ -574,7 +584,10 @@ export class SketchesServiceImpl
if (oldPath !== newPath) { if (oldPath !== newPath) {
await fs.rename(oldPath, newPath); await fs.rename(oldPath, newPath);
} }
await this.loadSketch(FileUri.create(destinationPath).toString()); // Sanity check. await this.doLoadSketch(
FileUri.create(destinationPath).toString(),
false
); // Sanity check.
resolve(); resolve();
} catch (e) { } catch (e) {
reject(e); reject(e);
@ -596,7 +609,7 @@ export class SketchesServiceImpl
} }
async archive(sketch: Sketch, destinationUri: string): Promise<string> { async archive(sketch: Sketch, destinationUri: string): Promise<string> {
await this.loadSketch(sketch.uri); // sanity check await this.doLoadSketch(sketch.uri, false); // sanity check
const { client } = await this.coreClient; const { client } = await this.coreClient;
const archivePath = FileUri.fsPath(destinationUri); const archivePath = FileUri.fsPath(destinationUri);
// The CLI cannot override existing archives, so we have to wipe it manually: https://github.com/arduino/arduino-cli/issues/1160 // 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
return this.tryParse(raw); return this.tryParse(raw);
} catch (err) { } catch (err) {
if ('code' in err && err.code === 'ENOENT') { if (ErrnoException.isENOENT(err)) {
return undefined; return undefined;
} }
throw err; throw err;
@ -695,7 +708,7 @@ export class SketchesServiceImpl
}); });
this.inoContent.resolve(inoContent); this.inoContent.resolve(inoContent);
} catch (err) { } catch (err) {
if ('code' in err && err.code === 'ENOENT') { if (ErrnoException.isENOENT(err)) {
// Ignored. The custom `.ino` blueprint file is optional. // Ignored. The custom `.ino` blueprint file is optional.
} else { } else {
throw err; throw err;
@ -763,7 +776,7 @@ async function isInvalidSketchNameError(
.map((name) => path.join(requestSketchPath, name))[0] .map((name) => path.join(requestSketchPath, name))[0]
); );
} catch (err) { } catch (err) {
if ('code' in err && err.code === 'ENOTDIR') { if (ErrnoException.isENOENT(err) || ErrnoException.isENOTDIR(err)) {
return undefined; return undefined;
} }
throw err; throw err;

View File

@ -0,0 +1,34 @@
export type ErrnoException = Error & { code: string; errno: number };
export namespace ErrnoException {
export function is(arg: unknown): arg is ErrnoException {
if (arg instanceof Error) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const error = arg as any;
return (
'code' in error &&
'errno' in error &&
typeof error['code'] === 'string' &&
typeof error['errno'] === 'number'
);
}
return false;
}
/**
* (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.
*/
export function isENOENT(
arg: unknown
): arg is ErrnoException & { code: 'ENOENT' } {
return is(arg) && arg.code === 'ENOENT';
}
/**
* (Not a directory): A component of the given pathname existed, but was not a directory as expected. Commonly raised by `fs.readdir`.
*/
export function isENOTDIR(
arg: unknown
): arg is ErrnoException & { code: 'ENOTDIR' } {
return is(arg) && arg.code === 'ENOTDIR';
}
}

View File

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