Skip to content

Commit

Permalink
check for appReadyMessage before previewing app
Browse files Browse the repository at this point in the history 
For apps that may output the server url before the app is ready, we need to check for the appReadyMessage before previewing the app.

Otherwise, the preview may fail because the app server is not ready to serve the app when the preview is requested.

In testing, fastapi apps were sometimes being previewed before the app was ready to serve the app.
  • Loading branch information
@sharon-wang
sharon-wang committed Nov 1, 2024
1 parent e842d83 commit 9bec9a3
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 20 deletions.
12 changes: 10 additions & 2 deletions extensions/positron-python/src/client/positron-run-app.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ export interface RunAppOptions {
* The optional URL path at which to preview the application.
*/
urlPath?: string;

/**
* The optional app ready message to wait for in the terminal before previewing the application.
*/
appReadyMessage?: string,
}

/**
Expand Down Expand Up @@ -78,9 +83,12 @@ export interface DebugAppOptions {
* The optional URL path at which to preview the application.
*/
urlPath?: string;
}

export interface DebugConfiguration {}
/**
* The optional app ready message to wait for in the terminal before previewing the application.
*/
appReadyMessage?: string,
}

/**
* The public API of the Positron Run App extension.
Expand Down
9 changes: 9 additions & 0 deletions extensions/positron-python/src/client/positron/webAppCommands.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export function activateWebAppCommands(serviceContainer: IServiceContainer, disp
'FastAPI',
(runtime, document, _urlPrefix) => getFastAPIDebugConfig(serviceContainer, runtime, document),
'/docs',
'Application startup complete',
),
registerExecCommand(Commands.Exec_Flask_In_Terminal, 'Flask', (_runtime, document, _urlPrefix) =>
getFlaskDebugConfig(document),
Expand All @@ -42,6 +43,8 @@ export function activateWebAppCommands(serviceContainer: IServiceContainer, disp
),
registerDebugCommand(Commands.Debug_FastAPI_In_Terminal, 'FastAPI', (runtime, document, _urlPrefix) =>
getFastAPIDebugConfig(serviceContainer, runtime, document),
'/docs',
'Application startup complete',
),
registerDebugCommand(Commands.Debug_Flask_In_Terminal, 'Flask', (_runtime, document, _urlPrefix) =>
getFlaskDebugConfig(document),
Expand All @@ -67,6 +70,7 @@ function registerExecCommand(
urlPrefix?: string,
) => DebugConfiguration | undefined | Promise<DebugConfiguration | undefined>,
urlPath?: string,
appReadyMessage?: string,
): vscode.Disposable {
return vscode.commands.registerCommand(command, async () => {
const runAppApi = await getPositronRunAppApi();
Expand Down Expand Up @@ -98,6 +102,7 @@ function registerExecCommand(
return terminalOptions;
},
urlPath,
appReadyMessage,
});
});
}
Expand All @@ -110,6 +115,8 @@ function registerDebugCommand(
document: vscode.TextDocument,
urlPrefix?: string,
) => DebugConfiguration | undefined | Promise<DebugConfiguration | undefined>,
urlPath?: string,
appReadyMessage?: string,
): vscode.Disposable {
return vscode.commands.registerCommand(command, async () => {
const runAppApi = await getPositronRunAppApi();
Expand All @@ -129,6 +136,8 @@ function registerDebugCommand(
stopOnEntry: false,
};
},
urlPath,
appReadyMessage,
});
});
}
Expand Down
89 changes: 73 additions & 16 deletions extensions/positron-run-app/src/api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ type PositronProxyInfo = {
finishProxySetup: (targetOrigin: string) => Promise<void>;
};

type AppPreviewOptions = {
proxyInfo?: PositronProxyInfo;
urlPath?: string;
appReadyMessage?: string;
};

export class PositronRunAppApiImpl implements PositronRunApp, vscode.Disposable {
private readonly _debugApplicationSequencerByName = new SequencerByKey<string>();
private readonly _debugApplicationDisposableByName = new Map<string, vscode.Disposable>();
Expand Down Expand Up @@ -162,7 +168,12 @@ export class PositronRunAppApiImpl implements PositronRunApp, vscode.Disposable
await this.setShellIntegrationSupported(true);

if (e.terminal === terminal) {
const didPreviewUrl = await previewUrlInExecutionOutput(e.execution, proxyInfo, options.urlPath);
const previewOptions: AppPreviewOptions = {
proxyInfo,
urlPath: options.urlPath,
appReadyMessage: options.appReadyMessage,
};
const didPreviewUrl = await previewUrlInExecutionOutput(e.execution, previewOptions);
if (didPreviewUrl) {
resolve(didPreviewUrl);
}
Expand Down Expand Up @@ -279,7 +290,12 @@ export class PositronRunAppApiImpl implements PositronRunApp, vscode.Disposable
await this.setShellIntegrationSupported(true);

if (await e.terminal.processId === processId) {
const didPreviewUrl = await previewUrlInExecutionOutput(e.execution, proxyInfo, options.urlPath);
const previewOptions: AppPreviewOptions = {
proxyInfo,
urlPath: options.urlPath,
appReadyMessage: options.appReadyMessage,
};
const didPreviewUrl = await previewUrlInExecutionOutput(e.execution, previewOptions);
if (didPreviewUrl) {
resolve(didPreviewUrl);
}
Expand Down Expand Up @@ -344,50 +360,91 @@ export class PositronRunAppApiImpl implements PositronRunApp, vscode.Disposable
}
}

async function previewUrlInExecutionOutput(execution: vscode.TerminalShellExecution, proxyInfo?: PositronProxyInfo, urlPath?: string) {
async function previewUrlInExecutionOutput(execution: vscode.TerminalShellExecution, options: AppPreviewOptions) {
// Wait for the server URL to appear in the terminal output, or a timeout.
const stream = execution.read();
const appReadyMessage = options.appReadyMessage?.trim();
const url = await raceTimeout(
(async () => {
// If an appReadyMessage is not provided, we'll consider the app ready as soon as the URL is found.
let appReady = !appReadyMessage;
let appUrl = undefined;
for await (const data of stream) {
log.warn('Execution:', execution.commandLine.value, data);
log.trace('Execution:', execution.commandLine.value, data);

// Ansi escape codes seem to mess up the regex match on Windows, so remove them first.
const dataCleaned = removeAnsiEscapeCodes(data);
const match = dataCleaned.match(localUrlRegex)?.[0];
if (match) {
return new URL(match.trim());

// Check if the app is ready, if it's not already ready and an appReadyMessage is provided.
if (!appReady && appReadyMessage) {
appReady = dataCleaned.includes(appReadyMessage);
if (appReady) {
log.debug(`App is ready - found appReadyMessage: '${appReadyMessage}'`);
// If the app URL was already found, we're done!
if (appUrl) {
return appUrl;
}
}
}

// Check if the app url is found in the terminal output.
if (!appUrl) {
const match = dataCleaned.match(localUrlRegex)?.[0];
if (match) {
appUrl = new URL(match.trim());
log.debug(`Found app URL in terminal output: ${appUrl.toString()}`);
// If the app is ready, we're done!
if (appReady) {
return appUrl;
}
}
}
}
log.warn('URL not found in terminal output');
return undefined;

// If we're here, we've reached the end of the stream without finding the app URL and/or
// the appReadyMessage.
if (!appReady) {
// It's possible that the app is ready, but the appReadyMessage was not found, for
// example, if the message has changed or was missed somehow. Log a warning.
log.warn(`Expected app ready message '${appReadyMessage}' not found in terminal`);
}
if (!appUrl) {
log.error('App URL not found in terminal output');
}
return appUrl;
})(),
15_000,
() => log.error('Timed out waiting for server output in terminal'),
);

if (url === undefined) {
log.warn('Timed out waiting for server URL in terminal output');
if (!url) {
log.error('Cannot preview URL. App is not ready or URL not found in terminal output.');
return false;
}

// Example: http://localhost:8500
const localBaseUri = vscode.Uri.parse(url.toString());

// Example: http://localhost:8500/url/path or http://localhost:8500
const localUri = urlPath ?
vscode.Uri.joinPath(localBaseUri, urlPath) : localBaseUri;
const localUri = options.urlPath ?
vscode.Uri.joinPath(localBaseUri, options.urlPath) : localBaseUri;

// Example: http://localhost:8080/proxy/5678/url/path or http://localhost:8080/proxy/5678
let previewUri = undefined;
if (proxyInfo) {
if (options.proxyInfo) {
// On Web (specifically Positron Server Web and not PWB), we need to set up the proxy with
// the urlPath appended to avoid issues where the app does not set the base url of the app
// or the base url of referenced assets correctly.
const applyWebPatch = isPositronWeb && !isRunningOnPwb;
const targetOrigin = applyWebPatch ? localUri.toString(true) : localBaseUri.toString();

log.debug(`Finishing proxy setup for app at ${targetOrigin}`);

// Finish the Positron proxy setup so that proxy middleware is hooked up.
await proxyInfo.finishProxySetup(targetOrigin);
previewUri = !applyWebPatch && urlPath ? vscode.Uri.joinPath(proxyInfo.externalUri, urlPath) : proxyInfo.externalUri;
await options.proxyInfo.finishProxySetup(targetOrigin);
previewUri = !applyWebPatch && options.urlPath
? vscode.Uri.joinPath(options.proxyInfo.externalUri, options.urlPath)
: options.proxyInfo.externalUri;
} else {
previewUri = await vscode.env.asExternalUri(localUri);
}
Expand Down
11 changes: 9 additions & 2 deletions extensions/positron-run-app/src/positron-run-app.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ export interface RunAppOptions {
* The optional URL path at which to preview the application.
*/
urlPath?: string;

/**
* The optional app ready message to wait for in the terminal before previewing the application.
*/
appReadyMessage?: string,
}

/**
Expand Down Expand Up @@ -78,9 +83,11 @@ export interface DebugAppOptions {
* The optional URL path at which to preview the application.
*/
urlPath?: string;
}

export interface DebugConfiguration {
/**
* The optional app ready message to wait for in the terminal before previewing the application.
*/
appReadyMessage?: string,
}

/**
Expand Down

0 comments on commit 9bec9a3

Please sign in to comment.