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

Website query API: Continue plugin installs on error #605

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
797dce0
Implement callback onBlueprintStepError to allow plugin installs to c…
eliot-akira Jul 19, 2023
71f32d9
Format
eliot-akira Jul 19, 2023
c6fbe1f
Export type OnStepError
eliot-akira Jul 19, 2023
38791f2
Format
eliot-akira Jul 19, 2023
71b2467
Catch resource fetch errors to let others continue to resolve
eliot-akira Jul 25, 2023
486a632
Website: Until we have a way to show notifications, output console er…
eliot-akira Jul 25, 2023
bc803cd
Define default onStepError callback that rethrows
eliot-akira Jul 25, 2023
5fb2d41
Add comment to clarify how a blueprint step's resources are resolved
eliot-akira Aug 5, 2023
1281355
Implement onRunningSteps for success/error handling per step; onSteps…
eliot-akira Aug 8, 2023
4681f81
Update types
eliot-akira Aug 8, 2023
4722601
Format
eliot-akira Aug 8, 2023
69eb4af
Website: Implement onRunningBlueprintSteps handler that allows plugin…
eliot-akira Aug 8, 2023
978a3fa
Merge from upstream/trunk and resolve conflict
eliot-akira Dec 11, 2023
c8467b1
Refactor as onStepProgress
eliot-akira Dec 11, 2023
06d7f26
Add blueprint step option `failMode` with value `abort` or `skip`, an…
eliot-akira Feb 8, 2024
df14575
Refactor `OnStepProgress` and `OnStepsCompleted` into `onStepError` a…
eliot-akira Feb 8, 2024
4243e59
For fail mode "skip", resolve step status instead of rejecting it on …
eliot-akira Feb 8, 2024
b32c27b
compileBlueprint option shouldBoot returns boolean
eliot-akira Feb 8, 2024
c7b7920
Use shared type StepFailMode
eliot-akira Feb 8, 2024
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
2 changes: 2 additions & 0 deletions packages/playground/blueprints/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ export type {
CompiledBlueprint,
CompileBlueprintOptions,
OnStepCompleted,
OnStepError,
ShouldBoot,
} from './lib/compile';
export type {
CachedResource,
Expand Down
132 changes: 113 additions & 19 deletions packages/playground/blueprints/src/lib/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import {
} from '@php-wasm/universal';
import type { SupportedPHPExtensionBundle } from '@php-wasm/universal';
import { FileReference, isFileReference, Resource } from './resources';
import { Step, StepDefinition } from './steps';
import {
Step,
StepDefinition,
StepFailMode,
defaultStepFailModes,
} from './steps';
import * as stepHandlers from './steps/handlers';
import { Blueprint } from './blueprint';

Expand Down Expand Up @@ -48,7 +53,11 @@ export interface CompiledBlueprint {
run: (playground: UniversalPHP) => Promise<void>;
}

export type OnStepCompleted = (output: any, step: StepDefinition) => any;
export type OnStepCompleted = (step: StepDefinition, result: any) => any;
export type OnStepError = (step: StepDefinition, error: any) => any;
export type ShouldBoot = (
completedStatus: Map<StepDefinition, boolean>
) => boolean;

export interface CompileBlueprintOptions {
/** Optional progress tracker to monitor progress */
Expand All @@ -57,8 +66,21 @@ export interface CompileBlueprintOptions {
semaphore?: Semaphore;
/** Optional callback with step output */
onStepCompleted?: OnStepCompleted;
onStepError?: OnStepError;
shouldBoot?: ShouldBoot;
}

/**
* Convert a settled Promise into a boolean indicating if it resolved or rejected.
*/
const isSettledPromiseResolved = <P extends Promise<any>>(
promise: P
): Promise<boolean> =>
promise.then(
() => true,
() => false
);

/**
* Compiles Blueprint into a form that can be executed.
*
Expand All @@ -73,6 +95,8 @@ export function compileBlueprint(
progress = new ProgressTracker(),
semaphore = new Semaphore({ concurrency: 3 }),
onStepCompleted = () => {},
onStepError,
shouldBoot,
}: CompileBlueprintOptions = {}
): CompiledBlueprint {
blueprint = {
Expand Down Expand Up @@ -171,33 +195,92 @@ export function compileBlueprint(
networking: blueprint.features?.networking ?? false,
},
run: async (playground: UniversalPHP) => {
const stepStatus = new Map<StepDefinition, Promise<void>>();
const stepPromises = new Map<
StepDefinition,
{
resolve: (result: any) => void;
reject: (error: any) => void;
}
>();
for (const { step, failMode } of compiled) {
const promise = new Promise<void>((resolve, reject) => {
stepPromises.set(step, { resolve, reject });
});
stepStatus.set(step, promise);
}

try {
// Start resolving resources early
for (const { resources } of compiled) {
for (const { resources, step, failMode } of compiled) {
const stepPromise = stepPromises.get(step);
for (const resource of resources) {
resource.setPlayground(playground);
if (resource.isAsync) {
resource.resolve();
resource.resolve().catch(function (error) {
dmsnell marked this conversation as resolved.
Show resolved Hide resolved
stepPromise?.reject(error);
});
}
}
}

for (const { run, step } of compiled) {
const result = await run(playground);
onStepCompleted(result, step);
/**
* When a compiled step's `run` method is called, it
* uses `resolveArguments` to await dependent resources.
*/
for (const { run, step, failMode } of compiled) {
const stepPromise = stepPromises.get(step);
let result;
try {
result = await run(playground);
} catch (error) {
stepPromise?.reject(result);
if (onStepError) {
onStepError(step, error);
} else if (failMode === 'abort') {
throw error;
}
/**
* Skip - This allows the default shouldBoot() below to
* consider the step completed.
*/
stepStatus.set(step, Promise.resolve());
continue;
}
stepPromise?.resolve(result);
onStepCompleted(step, result);
}
} finally {
try {
await (playground as any).goTo(
blueprint.landingPage || '/'
const completedStatus = new Map();
for (const { step } of compiled) {
completedStatus.set(
step,
await isSettledPromiseResolved(
Copy link
Collaborator

@adamziel adamziel Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this wait forever if the finally section is triggered by an error thrown before all the steps are run?

stepStatus.get(step) as Promise<any>
)
);
} catch (e) {
/*
* NodePHP exposes no goTo method.
* We can't use `goto` in playground here,
* because it may be a Comlink proxy object
* with no such method.
*/
}

// Either someone made a choice to boot or all steps succeeded.
const boot = shouldBoot
? shouldBoot(completedStatus)
: await isSettledPromiseResolved(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, no further steps are executed. Also, nothing seems to await stepPromises – do we need to reason about status as promises, then? Or could the status information be stored in a synchronous data structure like Map<StepDefinition, { didRun, error, result }>?

Promise.all([...stepStatus.values()])
);

if (boot) {
try {
await (playground as any)?.goTo(
blueprint.landingPage || '/'
);
} catch (e) {
/*
* NodePHP exposes no goTo method.
* We can't use `goto` in playground here,
* because it may be a Comlink proxy object
* with no such method.
*/
}
}
progress.finish();
}
Expand Down Expand Up @@ -327,7 +410,12 @@ function compileStep<S extends StepDefinition>(
rootProgressTracker,
totalProgressWeight,
}: CompileStepArgsOptions
): { run: CompiledStep; step: S; resources: Array<Resource> } {
): {
run: CompiledStep;
step: S;
resources: Array<Resource>;
failMode: StepFailMode;
} {
const stepProgress = rootProgressTracker.stage(
(step.progress?.weight || 1) / totalProgressWeight
);
Expand Down Expand Up @@ -373,7 +461,13 @@ function compileStep<S extends StepDefinition>(
resource.progress = stepProgress.stage(evenWeight);
}

return { run, step, resources };
return {
run,
step,
resources,
failMode:
(step.failMode || defaultStepFailModes.get(step.step)) ?? 'abort',
};
}

/**
Expand Down
8 changes: 8 additions & 0 deletions packages/playground/blueprints/src/lib/steps/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,15 @@ export type StepDefinition = Step & {
weight?: number;
caption?: string;
};
/**
* Option to decide what to do when a step fails:
* - `abort`: Stop the entire blueprint (default)
* - `skip`: Continue running the other steps
*/
failMode?: StepFailMode;
};
export type StepFailMode = 'abort' | 'skip';
export const defaultStepFailModes = new Map<string, StepFailMode>();

export { wpContentFilesExcludedFromExport } from './common';

Expand Down
6 changes: 6 additions & 0 deletions packages/playground/client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import {
Blueprint,
compileBlueprint,
OnStepCompleted,
OnStepError,
ShouldBoot,
runBlueprintSteps,
} from '@wp-playground/blueprints';
import { consumeAPI } from '@php-wasm/web';
Expand All @@ -46,6 +48,8 @@ export interface StartPlaygroundOptions {
disableProgressBar?: boolean;
blueprint?: Blueprint;
onBlueprintStepCompleted?: OnStepCompleted;
onBlueprintStepError?: OnStepError;
shouldBoot?: ShouldBoot;
}

/**
Expand All @@ -62,6 +66,7 @@ export async function startPlaygroundWeb({
progressTracker = new ProgressTracker(),
disableProgressBar,
onBlueprintStepCompleted,
onBlueprintStepError,
}: StartPlaygroundOptions): Promise<PlaygroundClient> {
assertValidRemote(remoteUrl);
allowStorageAccessByUserActivation(iframe);
Expand All @@ -76,6 +81,7 @@ export async function startPlaygroundWeb({
const compiled = compileBlueprint(blueprint, {
progress: progressTracker.stage(0.5),
onStepCompleted: onBlueprintStepCompleted,
onStepError: onBlueprintStepError,
});
const playground = await doStartPlaygroundWeb(
iframe,
Expand Down
16 changes: 16 additions & 0 deletions packages/playground/website/src/lib/hooks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useEffect, useRef, useState } from 'react';
import { Blueprint, startPlaygroundWeb } from '@wp-playground/client';
import type { PlaygroundClient } from '@wp-playground/client';
import type { Step, CorePluginReference } from '@wp-playground/blueprints';
import { getRemoteUrl } from './config';

interface UsePlaygroundOptions {
Expand Down Expand Up @@ -40,10 +41,25 @@ export function usePlayground({ blueprint, storage }: UsePlaygroundOptions) {
remoteUrl.searchParams.set('storage', storage);
}

const onBlueprintStepError = (step: Step, error: Error) => {
if (step.step === 'installPlugin') {
// TODO: Open modal to display error message
alert(
`Failed to install: ${
(step.pluginZipFile as CorePluginReference).slug ||
Copy link
Collaborator Author

@eliot-akira eliot-akira Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type coersion to CorePluginReference is questionable. It was a temporary workaround because step.pluginZipFile is a FileReference, a union of several types which have property slug, name, path, or nothing for the name.

'plugin'
}`
);
}

// Fall through to step option failMode to abort or skip
};

startPlaygroundWeb({
iframe,
remoteUrl: remoteUrl.toString(),
blueprint,
onBlueprintStepError,
}).then(async (playground) => {
playground.onNavigation((url) => setUrl(url));
setPlayground(() => playground);
Expand Down
8 changes: 6 additions & 2 deletions packages/playground/wordpress/build/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,9 @@ versions = Object.keys(versions)
// Write the updated JSON back to the file
await fs.writeFile(versionsPath, JSON.stringify(versions, null, 2));

const latestStableVersion = Object.keys(versions).filter((v) => v.match(/^\d/))[0];
const latestStableVersion = Object.keys(versions).filter((v) =>
v.match(/^\d/)
)[0];

// Refresh get-wordpress-module.ts
const getWordPressModulePath = `${outputJsDir}/get-wordpress-module.ts`;
Expand All @@ -186,7 +188,9 @@ const getWordPressModuleContent = `
* This file must statically exists in the project because of the way
* vite resolves imports.
*/
export function getWordPressModule(wpVersion: string = ${JSON.stringify(latestStableVersion)}) {
export function getWordPressModule(wpVersion: string = ${JSON.stringify(
latestStableVersion
)}) {
switch (wpVersion) {
${Object.keys(versions)
.map(
Expand Down