From 12fff33d9a7809d700983b1df28e2c823248941d Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 2 Nov 2022 14:47:52 +0100 Subject: [PATCH 1/3] feat: generalized Node.js error handling gracefully handle when sketch folder has been deleted Closes #1596 Signed-off-by: Akos Kitta --- .../theia/electron-main-application.ts | 5 +-- .../src/node/arduino-daemon-impl.ts | 3 +- .../src/node/config-service-impl.ts | 3 +- .../src/node/sketches-service-impl.ts | 9 ++--- .../src/node/utils/errors.ts | 34 +++++++++++++++++++ 5 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 arduino-ide-extension/src/node/utils/errors.ts diff --git a/arduino-ide-extension/src/electron-main/theia/electron-main-application.ts b/arduino-ide-extension/src/electron-main/theia/electron-main-application.ts index 335899d70..61bb26cc4 100644 --- a/arduino-ide-extension/src/electron-main/theia/electron-main-application.ts +++ b/arduino-ide-extension/src/electron-main/theia/electron-main-application.ts @@ -28,6 +28,7 @@ import { SHOW_PLOTTER_WINDOW, } from '../../common/ipc-communication'; import isValidPath = require('is-valid-path'); +import { ErrnoException } from '../../node/utils/errors'; app.commandLine.appendSwitch('disable-http-cache'); @@ -172,7 +173,7 @@ export class ElectronMainApplication extends TheiaElectronMainApplication { try { stats = await fs.stat(path); } catch (err) { - if ('code' in err && err.code === 'ENOENT') { + if (ErrnoException.isENOENT(err)) { return undefined; } throw err; @@ -215,7 +216,7 @@ export class ElectronMainApplication extends TheiaElectronMainApplication { const resolved = await fs.realpath(resolve(cwd, maybePath)); return resolved; } catch (err) { - if ('code' in err && err.code === 'ENOENT') { + if (ErrnoException.isENOENT(err)) { return undefined; } throw err; diff --git a/arduino-ide-extension/src/node/arduino-daemon-impl.ts b/arduino-ide-extension/src/node/arduino-daemon-impl.ts index 4a3ad882c..d4b345c4e 100644 --- a/arduino-ide-extension/src/node/arduino-daemon-impl.ts +++ b/arduino-ide-extension/src/node/arduino-daemon-impl.ts @@ -16,6 +16,7 @@ import { BackendApplicationContribution } from '@theia/core/lib/node/backend-app import { ArduinoDaemon, NotificationServiceServer } from '../common/protocol'; import { CLI_CONFIG } from './cli-config'; import { getExecPath, spawnCommand } from './exec-util'; +import { ErrnoException } from './utils/errors'; @injectable() export class ArduinoDaemonImpl @@ -184,7 +185,7 @@ export class ArduinoDaemonImpl } return false; } catch (error) { - if ('code' in error && error.code === 'ENOENT') { + if (ErrnoException.isENOENT(error)) { return false; } throw error; diff --git a/arduino-ide-extension/src/node/config-service-impl.ts b/arduino-ide-extension/src/node/config-service-impl.ts index fa2e259c3..2efc4e774 100644 --- a/arduino-ide-extension/src/node/config-service-impl.ts +++ b/arduino-ide-extension/src/node/config-service-impl.ts @@ -26,6 +26,7 @@ import { DefaultCliConfig, CLI_CONFIG } from './cli-config'; import { Deferred } from '@theia/core/lib/common/promise-util'; import { EnvVariablesServer } from '@theia/core/lib/common/env-variables'; import { deepClone } from '@theia/core'; +import { ErrnoException } from './utils/errors'; const deepmerge = require('deepmerge'); @@ -146,7 +147,7 @@ export class ConfigServiceImpl const fallbackModel = await this.getFallbackCliConfig(); return deepmerge(fallbackModel, model) as DefaultCliConfig; } catch (error) { - if ('code' in error && error.code === 'ENOENT') { + if (ErrnoException.isENOENT(error)) { if (initializeIfAbsent) { await this.initCliConfigTo(dirname(cliConfigPath)); return this.loadCliConfig(false); diff --git a/arduino-ide-extension/src/node/sketches-service-impl.ts b/arduino-ide-extension/src/node/sketches-service-impl.ts index 9cbee96c4..22c534f04 100644 --- a/arduino-ide-extension/src/node/sketches-service-impl.ts +++ b/arduino-ide-extension/src/node/sketches-service-impl.ts @@ -33,6 +33,7 @@ import { TempSketchPrefix, } from './is-temp-sketch'; import { join } from 'path'; +import { ErrnoException } from './utils/errors'; const RecentSketches = 'recent-sketches.json'; const DefaultIno = `void setup() { @@ -278,7 +279,7 @@ export class SketchesServiceImpl ); } } catch (err) { - if ('code' in err && err.code === 'ENOENT') { + if (ErrnoException.isENOENT(err)) { this.logger.debug( `<<< '${RecentSketches}' does not exist yet. This is normal behavior. Falling back to empty data.` ); @@ -666,7 +667,7 @@ export class SketchesServiceImpl return this.tryParse(raw); } catch (err) { - if ('code' in err && err.code === 'ENOENT') { + if (ErrnoException.isENOENT(err)) { return undefined; } throw err; @@ -695,7 +696,7 @@ export class SketchesServiceImpl }); this.inoContent.resolve(inoContent); } catch (err) { - if ('code' in err && err.code === 'ENOENT') { + if (ErrnoException.isENOENT(err)) { // Ignored. The custom `.ino` blueprint file is optional. } else { throw err; @@ -763,7 +764,7 @@ async function isInvalidSketchNameError( .map((name) => path.join(requestSketchPath, name))[0] ); } catch (err) { - if ('code' in err && err.code === 'ENOTDIR') { + if (ErrnoException.isENOENT(err) || ErrnoException.isENOTDIR(err)) { return undefined; } throw err; diff --git a/arduino-ide-extension/src/node/utils/errors.ts b/arduino-ide-extension/src/node/utils/errors.ts new file mode 100644 index 000000000..952c43763 --- /dev/null +++ b/arduino-ide-extension/src/node/utils/errors.ts @@ -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'; + } +} From 74d93f4ea0c6fedc92af01e8ebc49023045daac7 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 2 Nov 2022 14:48:02 +0100 Subject: [PATCH 2/3] fix: typo Signed-off-by: Akos Kitta --- scripts/i18n/transifex-pull.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/i18n/transifex-pull.js b/scripts/i18n/transifex-pull.js index 4f1f3fcc3..e620e88da 100644 --- a/scripts/i18n/transifex-pull.js +++ b/scripts/i18n/transifex-pull.js @@ -54,7 +54,7 @@ const requestTranslationDownload = async (relationships) => { const getTranslationDownloadStatus = async (language, downloadRequestId) => { // 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; while (true) { const url = transifex.url( From 76a58899b4642168a6b597140582fba6431b4593 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 2 Nov 2022 14:50:01 +0100 Subject: [PATCH 3/3] feat: spare detecting invalid sketch name error The invalid sketch name detection requires at least one extra FS access. Do not try to detect invalid sketch name error, but use the original `NotFound` from the CLI. Signed-off-by: Akos Kitta --- .../src/node/sketches-service-impl.ts | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/arduino-ide-extension/src/node/sketches-service-impl.ts b/arduino-ide-extension/src/node/sketches-service-impl.ts index 22c534f04..1d1d61221 100644 --- a/arduino-ide-extension/src/node/sketches-service-impl.ts +++ b/arduino-ide-extension/src/node/sketches-service-impl.ts @@ -188,6 +188,13 @@ export class SketchesServiceImpl } async loadSketch(uri: string): Promise { + return this.doLoadSketch(uri); + } + + private async doLoadSketch( + uri: string, + detectInvalidSketchNameError = true + ): Promise { const { client, instance } = await this.coreClient; const req = new LoadSketchRequest(); const requestSketchPath = FileUri.fsPath(uri); @@ -202,17 +209,19 @@ export class SketchesServiceImpl if (err) { let rejectWith: unknown = err; if (isNotFoundError(err)) { - const invalidMainSketchFilePath = await isInvalidSketchNameError( - err, - requestSketchPath - ); - if (invalidMainSketchFilePath) { - rejectWith = SketchesError.InvalidName( - err.details, - FileUri.create(invalidMainSketchFilePath).toString() + rejectWith = SketchesError.NotFound(err.details, uri); + // detecting the invalid sketch name error is not for free as it requires multiple filesystem access. + if (detectInvalidSketchNameError) { + const invalidMainSketchFilePath = await isInvalidSketchNameError( + err, + requestSketchPath ); - } else { - rejectWith = SketchesError.NotFound(err.details, uri); + if (invalidMainSketchFilePath) { + rejectWith = SketchesError.InvalidName( + err.details, + FileUri.create(invalidMainSketchFilePath).toString() + ); + } } } reject(rejectWith); @@ -318,7 +327,7 @@ export class SketchesServiceImpl let sketch: Sketch | undefined = undefined; try { - sketch = await this.loadSketch(uri); + sketch = await this.doLoadSketch(uri, false); this.logger.debug( `Loaded sketch ${JSON.stringify( sketch @@ -391,7 +400,7 @@ export class SketchesServiceImpl )) { let sketch: SketchWithDetails | undefined = undefined; try { - sketch = await this.loadSketch(uri); + sketch = await this.doLoadSketch(uri, false); } catch {} if (!sketch) { needsUpdate = true; @@ -414,14 +423,14 @@ export class SketchesServiceImpl async cloneExample(uri: string): Promise { const [sketch, parentPath] = await Promise.all([ - this.loadSketch(uri), + this.doLoadSketch(uri, false), this.createTempFolder(), ]); const destinationUri = FileUri.create( path.join(parentPath, sketch.name) ).toString(); const copiedSketchUri = await this.copy(sketch, { destinationUri }); - return this.loadSketch(copiedSketchUri); + return this.doLoadSketch(copiedSketchUri, false); } async createNewSketch(): Promise { @@ -478,7 +487,7 @@ export class SketchesServiceImpl fs.mkdir(sketchDir, { recursive: true }), ]); await fs.writeFile(sketchFile, inoContent, { encoding: 'utf8' }); - return this.loadSketch(FileUri.create(sketchDir).toString()); + return this.doLoadSketch(FileUri.create(sketchDir).toString(), false); } /** @@ -529,7 +538,7 @@ export class SketchesServiceImpl uri: string ): Promise { try { - const sketch = await this.loadSketch(uri); + const sketch = await this.doLoadSketch(uri, false); return sketch; } catch (err) { if (SketchesError.NotFound.is(err) || SketchesError.InvalidName.is(err)) { @@ -554,7 +563,7 @@ export class SketchesServiceImpl } // Nothing to do when source and destination are the same. if (sketch.uri === destinationUri) { - await this.loadSketch(sketch.uri); // Sanity check. + await this.doLoadSketch(sketch.uri, false); // Sanity check. return sketch.uri; } @@ -575,7 +584,10 @@ export class SketchesServiceImpl if (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(); } catch (e) { reject(e); @@ -597,7 +609,7 @@ export class SketchesServiceImpl } async archive(sketch: Sketch, destinationUri: string): Promise { - await this.loadSketch(sketch.uri); // sanity check + await this.doLoadSketch(sketch.uri, false); // sanity check const { client } = await this.coreClient; 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