Skip to content

Commit

Permalink
feat(web): add error handling and tests
Browse files Browse the repository at this point in the history
Addresses code review comments.
  • Loading branch information
ermshiperete committed Sep 12, 2024
1 parent cbbe971 commit 1dd6989
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 116 deletions.
4 changes: 2 additions & 2 deletions web/src/engine/keyboard/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ export * from "./keyboards/defaultLayouts.js";
export { default as Keyboard } from "./keyboards/keyboard.js";
export * from "./keyboards/keyboard.js";
export { KeyboardHarness, KeyboardKeymanGlobal, MinimalCodesInterface, MinimalKeymanGlobal } from "./keyboards/keyboardHarness.js";
export { default as KeyboardLoaderBase, } from "./keyboards/keyboardLoaderBase.js";
export { KeyboardLoadErrorBuilder, KeyboardMissingError, KeyboardScriptError } from './keyboards/keyboardLoadError.js'
export { KeyboardLoaderBase } from "./keyboards/keyboardLoaderBase.js";
export { KeyboardLoadErrorBuilder, KeyboardMissingError, KeyboardScriptError, KeyboardDownloadError } from './keyboards/keyboardLoadError.js'
export {
CloudKeyboardFont,
internalizeFont,
Expand Down
27 changes: 27 additions & 0 deletions web/src/engine/keyboard/src/keyboards/keyboardLoadError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { type KeyboardStub } from './keyboardLoaderBase.js';
export interface KeyboardLoadErrorBuilder {
scriptError(err?: Error): void;
missingError(err: Error): void;
missingKeyboardError(msg: string, err: Error): void;
keyboardDownloadError(msg: string, err: Error): void;
}

export class KeyboardScriptError extends Error {
Expand All @@ -23,6 +25,15 @@ export class KeyboardMissingError extends Error {
}
}

export class KeyboardDownloadError extends Error {
public readonly cause;

constructor(message: string, cause?: Error) {
super(message);
this.cause = cause;
}
}

export class UriBasedErrorBuilder implements KeyboardLoadErrorBuilder {
readonly uri: string;

Expand All @@ -35,10 +46,18 @@ export class UriBasedErrorBuilder implements KeyboardLoadErrorBuilder {
return new KeyboardMissingError(msg, err);
}

missingKeyboardError(msg: string, err: Error) {
return new KeyboardMissingError(msg, err);
}

scriptError(err: Error) {
const msg = `Error registering the keyboard script at ${this.uri}; it may contain an error.`;
return new KeyboardScriptError(msg, err);
}

keyboardDownloadError(msg: string, err: Error) {
return new KeyboardDownloadError(msg, err);
}
}

export class StubBasedErrorBuilder implements KeyboardLoadErrorBuilder {
Expand All @@ -54,10 +73,18 @@ export class StubBasedErrorBuilder implements KeyboardLoadErrorBuilder {
return new KeyboardMissingError(msg, err);
}

missingKeyboardError(msg: string, err: Error) {
return new KeyboardMissingError(msg, err);
}

scriptError(err: Error) {
const stub = this.stub;
const msg = `Error registering the ${stub.name} keyboard for ${stub.langName}; keyboard script at ${stub.filename} may contain an error.`;
return new KeyboardScriptError(msg, err);
}

keyboardDownloadError(msg: string, err: Error) {
return new KeyboardDownloadError(msg, err);
}
}

26 changes: 22 additions & 4 deletions web/src/engine/keyboard/src/keyboards/keyboardLoaderBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { KeyboardLoadErrorBuilder, StubBasedErrorBuilder, UriBasedErrorBuilder }

export type KeyboardStub = KeyboardProperties & { filename: string };

export default abstract class KeyboardLoaderBase {
export abstract class KeyboardLoaderBase {
private _harness: KeyboardHarness;

public get harness(): KeyboardHarness {
Expand All @@ -16,20 +16,38 @@ export default abstract class KeyboardLoaderBase {
this._harness = harness;
}

/**
* Load a keyboard from a remote or local URI.
*
* @param uri The URI of the keyboard to load.
* @returns A Promise that resolves to the loaded keyboard.
*/
public loadKeyboardFromPath(uri: string): Promise<Keyboard> {
this.harness.install();
return this.loadKeyboardInternal(uri, new UriBasedErrorBuilder(uri));
}

/**
* Load a keyboard from keyboard stub.
*
* @param stub The stub of the keyboard to load.
* @returns A Promise that resolves to the loaded keyboard.
*/
public loadKeyboardFromStub(stub: KeyboardStub) {
this.harness.install();
return this.loadKeyboardInternal(stub.filename, new StubBasedErrorBuilder(stub));
}

private async loadKeyboardInternal(uri: string, errorBuilder: KeyboardLoadErrorBuilder): Promise<Keyboard> {
const blob = await this.loadKeyboardBlob(uri);
const blob = await this.loadKeyboardBlob(uri, errorBuilder);

let script: string;
try {
script = await blob.text();
} catch (e) {
throw errorBuilder.missingKeyboardError('The keyboard has an invalid encoding.', e);
}

const script = await blob.text();
if (script.startsWith('KXTS', 0)) {
// KMX or LDML (KMX+) keyboard
console.error("KMX keyboard loading is not yet implemented!");
Expand All @@ -40,7 +58,7 @@ export default abstract class KeyboardLoaderBase {
return await this.loadKeyboardFromScript(script, errorBuilder);
}

protected abstract loadKeyboardBlob(uri: string): Promise<Blob>;
protected abstract loadKeyboardBlob(uri: string, errorBuilder: KeyboardLoadErrorBuilder): Promise<Blob>;

protected abstract loadKeyboardFromScript(scriptSrc: string, errorBuilder: KeyboardLoadErrorBuilder): Promise<Keyboard>;
}
29 changes: 22 additions & 7 deletions web/src/engine/keyboard/src/keyboards/loaders/domKeyboardLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import { default as Keyboard } from '../keyboard.js';
import { KeyboardHarness, MinimalKeymanGlobal } from '../keyboardHarness.js';
import { default as KeyboardLoaderBase } from '../keyboardLoaderBase.js';
import { KeyboardLoadErrorBuilder, KeyboardMissingError } from '../keyboardLoadError.js';
import { KeyboardLoaderBase } from '../keyboardLoaderBase.js';
import { KeyboardLoadErrorBuilder } from '../keyboardLoadError.js';

export class DOMKeyboardLoader extends KeyboardLoaderBase {
public readonly element: HTMLIFrameElement;
Expand All @@ -29,20 +29,35 @@ export class DOMKeyboardLoader extends KeyboardLoaderBase {
this.performCacheBusting = cacheBust || false;
}

protected async loadKeyboardBlob(uri: string): Promise<Blob> {
protected async loadKeyboardBlob(uri: string, errorBuilder: KeyboardLoadErrorBuilder): Promise<Blob> {
if (this.performCacheBusting) {
uri = this.cacheBust(uri);
}

const response = await fetch(uri);
let response: Response;
try {
response = await fetch(uri);
} catch (e) {
throw errorBuilder.keyboardDownloadError(`Unable to fetch fetch keyboard at ${uri}`, e);
}

if (!response.ok) {
throw new KeyboardMissingError(`Cannot find the keyboard at ${uri}.`, new Error(`HTTP ${response.status} ${response.statusText}`));
throw errorBuilder.missingError(new Error(`HTTP ${response.status} ${response.statusText}`));
}

try {
return await response.blob();
} catch (e) {
throw errorBuilder.keyboardDownloadError(`Unable to retrieve blob from keyboard at ${uri}`, e);
}
return response.blob();
}

protected async loadKeyboardFromScript(script: string, errorBuilder: KeyboardLoadErrorBuilder): Promise<Keyboard> {
this.evalScriptInContext(script, this.harness._jsGlobal);
try {
this.evalScriptInContext(script, this.harness._jsGlobal);
} catch (e) {
throw errorBuilder.scriptError(e);
}
const keyboard = this.harness.loadedKeyboard;
this.harness.loadedKeyboard = null;
return keyboard;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import vm from 'node:vm';
import { readFile } from 'node:fs/promises';

import { globalObject } from '@keymanapp/web-utils';

import { default as Keyboard } from '../keyboard.js';
import { KeyboardHarness, MinimalKeymanGlobal } from '../keyboardHarness.js';
import { default as KeyboardLoaderBase } from '../keyboardLoaderBase.js';
import { KeyboardLoaderBase } from '../keyboardLoaderBase.js';
import { KeyboardLoadErrorBuilder } from '../keyboardLoadError.js';

import vm from 'vm';
import { readFile } from 'fs/promises';
import { globalObject } from '@keymanapp/web-utils';

export class NodeKeyboardLoader extends KeyboardLoaderBase {
constructor()
constructor(harness: KeyboardHarness);
Expand All @@ -25,13 +26,18 @@ export class NodeKeyboardLoader extends KeyboardLoaderBase {
}
}

protected async loadKeyboardBlob(uri: string): Promise<Blob> {
protected async loadKeyboardBlob(uri: string, errorBuilder: KeyboardLoadErrorBuilder): Promise<Blob> {
// `fs` does not like 'file:///'; it IS "File System" oriented, after all, and wants a path, not a URI.
if (uri.indexOf('file:///') == 0) {
uri = uri.substring('file:///'.length);
}

const buffer = await readFile(uri);
let buffer: Buffer;
try {
buffer = await readFile(uri);
} catch (err) {
throw errorBuilder.keyboardDownloadError(`Unable to read keyboard file at ${uri}`, err);
}
return new Blob([buffer]);
}

Expand All @@ -40,12 +46,12 @@ export class NodeKeyboardLoader extends KeyboardLoaderBase {
try {
script = new vm.Script(scriptSrc);
} catch (err) {
return Promise.reject(errorBuilder.missingError(err));
throw errorBuilder.missingError(err);
}
try {
script.runInContext(this.harness._jsGlobal);
} catch (err) {
return Promise.reject(errorBuilder.scriptError(err));
throw errorBuilder.scriptError(err);
}

const keyboard = this.harness.loadedKeyboard;
Expand Down
94 changes: 0 additions & 94 deletions web/src/test/auto/headless/engine/keyboard/keyboard-loading.js

This file was deleted.

Loading

0 comments on commit 1dd6989

Please sign in to comment.