Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Handle gracefully when trying to detect invalid sketch name error and folder is missing on filesystem #1616

Merged
merged 3 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion arduino-ide-extension/src/node/arduino-daemon-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion arduino-ide-extension/src/node/config-service-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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);
Expand Down
59 changes: 36 additions & 23 deletions arduino-ide-extension/src/node/sketches-service-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -187,6 +188,13 @@ export class SketchesServiceImpl
}

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 req = new LoadSketchRequest();
const requestSketchPath = FileUri.fsPath(uri);
Expand All @@ -201,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);
Expand Down Expand Up @@ -278,7 +288,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.`
);
Expand Down Expand Up @@ -317,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
Expand Down Expand Up @@ -390,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;
Expand All @@ -413,14 +423,14 @@ export class SketchesServiceImpl

async cloneExample(uri: string): Promise<Sketch> {
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<Sketch> {
Expand Down Expand Up @@ -477,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);
}

/**
Expand Down Expand Up @@ -528,7 +538,7 @@ export class SketchesServiceImpl
uri: string
): Promise<SketchWithDetails | undefined> {
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)) {
Expand All @@ -553,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;
}

Expand All @@ -574,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);
Expand All @@ -596,7 +609,7 @@ export class SketchesServiceImpl
}

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 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
Expand Down Expand Up @@ -666,7 +679,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;
Expand Down Expand Up @@ -695,7 +708,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;
Expand Down Expand Up @@ -763,7 +776,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;
Expand Down
34 changes: 34 additions & 0 deletions arduino-ide-extension/src/node/utils/errors.ts
Original file line number Diff line number Diff line change
@@ -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';
}
}
2 changes: 1 addition & 1 deletion scripts/i18n/transifex-pull.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down