skip Positron Proxy for fastapi and streamlit apps on PWB #5138
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,9 @@ import { raceTimeout, removeAnsiEscapeCodes, SequencerByKey } from './utils'; | |
| // Regex to match a URL with the format http://localhost:1234/path | ||
| const localUrlRegex = /http:\/\/(localhost|127\.0\.0\.1):(\d{1,5})(\/[^\s]*)?/; | ||
|
|
||
| const isPositronWeb = vscode.env.uiKind === vscode.UIKind.Web; | ||
| const isRunningOnPwb = !!process.env.RS_SERVER_URL && isPositronWeb; | ||
|
|
||
| type PositronProxyInfo = { | ||
| proxyPath: string; | ||
| externalUri: vscode.Uri; | ||
|
|
@@ -88,12 +91,18 @@ export class PositronRunAppApiImpl implements PositronRunApp, vscode.Disposable | |
| return; | ||
| } | ||
|
|
||
| // Set up the proxy server for the application if applicable. | ||
| let urlPrefix = undefined; | ||
| let proxyInfo: PositronProxyInfo | undefined; | ||
| if (shouldUsePositronProxy(options.name)) { | ||
| // Start the proxy server | ||
| proxyInfo = await vscode.commands.executeCommand<PositronProxyInfo>('positronProxy.startPendingProxyServer'); | ||
| log.debug(`Proxy started for app at proxy path ${proxyInfo.proxyPath} with uri ${proxyInfo.externalUri.toString()}`); | ||
| urlPrefix = proxyInfo.proxyPath; | ||
| } | ||
|
|
||
| // Get the terminal options for the application. | ||
| progress.report({ message: vscode.l10n.t('Getting terminal options...') }); | ||
| const terminalOptions = await options.getTerminalOptions(runtime, document, urlPrefix); | ||
| if (!terminalOptions) { | ||
| return; | ||
|
|
@@ -221,11 +230,18 @@ export class PositronRunAppApiImpl implements PositronRunApp, vscode.Disposable | |
| return; | ||
| } | ||
|
|
||
| // Set up the proxy server for the application if applicable. | ||
| let urlPrefix = undefined; | ||
| let proxyInfo: PositronProxyInfo | undefined; | ||
| if (shouldUsePositronProxy(options.name)) { | ||
| // Start the proxy server | ||
| proxyInfo = await vscode.commands.executeCommand<PositronProxyInfo>('positronProxy.startPendingProxyServer'); | ||
| log.debug(`Proxy started for app at proxy path ${proxyInfo.proxyPath} with uri ${proxyInfo.externalUri.toString()}`); | ||
| urlPrefix = proxyInfo.proxyPath; | ||
| } | ||
|
|
||
| // Get the debug config for the application. | ||
| progress.report({ message: vscode.l10n.t('Getting debug configuration...') }); | ||
| const debugConfig = await options.getDebugConfiguration(runtime, document, urlPrefix); | ||
| if (!debugConfig) { | ||
| return; | ||
|
|
@@ -328,7 +344,7 @@ export class PositronRunAppApiImpl implements PositronRunApp, vscode.Disposable | |
| } | ||
| } | ||
|
|
||
| async function previewUrlInExecutionOutput(execution: vscode.TerminalShellExecution, proxyInfo?: PositronProxyInfo, urlPath?: string) { | ||
| // Wait for the server URL to appear in the terminal output, or a timeout. | ||
| const stream = execution.read(); | ||
| const url = await raceTimeout( | ||
|
|
@@ -353,18 +369,33 @@ async function previewUrlInExecutionOutput(execution: vscode.TerminalShellExecut | |
| 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; | ||
|
|
||
| // Example: http://localhost:8080/proxy/5678/url/path or http://localhost:8080/proxy/5678 | ||
| let previewUri = undefined; | ||
| if (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(); | ||
|
|
||
| // 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; | ||
| } else { | ||
| previewUri = await vscode.env.asExternalUri(localUri); | ||
| } | ||
|
|
||
| log.debug(`Viewing app at local uri: ${localUri.toString(true)} with external uri ${previewUri.toString(true)}`); | ||
|
|
||
| // Preview the app in the Viewer. | ||
| positron.window.previewUrl(previewUri); | ||
|
There was a problem hiding this comment. @seeM When running Fastapi in Positron on Workbench, sometimes the app url is previewed in the Viewer slightly too early, before the app server is ready. To get around this, a user can refresh the Viewer, but that is not ideal if it happens every time. Do you have thoughts on how to get around this issue? Maybe we can ping the uri to check if it's responsive before displaying it in the Viewer? There was a problem hiding this comment. Maybe #4276 is describing what I'm seeing? There was a problem hiding this comment. I assume that uvicorn would only print the URL to stdout once the server is ready to receive connections (I haven't confirmed that). Is the delay happening at the Positron Proxy layer? If so, maybe there's something we could expose from the Positron Proxy extension i.e. an There was a problem hiding this comment. I looked into this a bit more and it does not seem to be in the Positron Proxy layer, as the issue still occurs even if we don't use the Positron Proxy. For fastapi with uvicorn, we may want to wait for Perhaps this would be a separate property in the terminal/debug config for some terminal text that indicates the app is ready, before we extract the app url and view it. I'll open a separate issue for this. |
||
|
|
||
| return true; | ||
| } | ||
|
|
@@ -442,3 +473,27 @@ async function showShellIntegrationNotSupportedMessage(): Promise<void> { | |
| await runAppConfig.update('showShellIntegrationNotSupportedMessage', false, vscode.ConfigurationTarget.Global); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Check if the Positron proxy should be used for the given app. | ||
| * Generally, we should avoid skipping the proxy unless there is a good reason to do so, as the | ||
| * proxy gives us the ability to intercept requests and responses to the app, which is useful for | ||
| * things like debugging, applying styling or fixing up urls. | ||
| * @param appName The name of the app; indicated in extensions/positron-python/src/client/positron/webAppCommands.ts | ||
| * @returns Whether to use the Positron proxy for the app. | ||
| */ | ||
| function shouldUsePositronProxy(appName: string) { | ||
| switch (appName.trim().toLowerCase()) { | ||
| // Streamlit apps don't work in Positron on Workbench with SSL enabled when run through the proxy. | ||
| case 'streamlit': | ||
| // FastAPI apps don't work in Positron on Workbench when run through the proxy. | ||
| case 'fastapi': | ||
| if (isRunningOnPwb) { | ||
| return false; | ||
| } | ||
| return true; | ||
| default: | ||
| // By default, proxy the app. | ||
| return true; | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this allows us to run
vsce packagein theextensions/positron-run-appdirectory to generate a VSIX, which can be loaded into Positron. This is helpful for faster extension dev iteration when running a pre-built reh-web version of Positron.