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

skip Positron Proxy for fastapi and streamlit apps on PWB #5138

Merged
merged 21 commits into from
Oct 29, 2024
Merged
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f260290
add log output channel for positron proxy
sharon-wang Oct 23, 2024
7cc22a9
fix handling of custom urlPath in positron-run-app
sharon-wang Oct 23, 2024
aaf999b
remove trailing slashes and add trace logging
sharon-wang Oct 23, 2024
a5d3c2f
add workbench-specific handling to determine the proxy targetOrigin
sharon-wang Oct 23, 2024
c5aca3c
remove removal of trailing slash from target origin
sharon-wang Oct 23, 2024
e817274
remove fastapi workbench-specific workaround
sharon-wang Oct 28, 2024
ec92a71
skip positron proxy for fastapi apps on PWB
sharon-wang Oct 28, 2024
8cf5740
remove fastapi workbench-specific workaround
sharon-wang Oct 28, 2024
9cea3be
update function doc and match style with other functions
sharon-wang Oct 28, 2024
88696b2
add typescript dep to positron-run-app
sharon-wang Oct 28, 2024
1528565
add websocket proxy req logging and explicit upgrade handling
sharon-wang Oct 24, 2024
a2b51ae
override websocket proxy request origin header
sharon-wang Oct 24, 2024
b91c7f5
WIP: extra handling in positron proxy for websockets
sharon-wang Oct 25, 2024
b67a1b5
add debug logging for websocket upgrades
sharon-wang Oct 25, 2024
c8ce6a1
try to rewrite the proxy with http-proxy
sharon-wang Oct 25, 2024
d142b12
Revert "try to rewrite the proxy with http-proxy"
sharon-wang Oct 29, 2024
df57747
Revert "add debug logging for websocket upgrades"
sharon-wang Oct 29, 2024
48d38f8
Revert "WIP: extra handling in positron proxy for websockets"
sharon-wang Oct 29, 2024
8f4713d
Revert "override websocket proxy request origin header"
sharon-wang Oct 29, 2024
465e2f0
Revert "add websocket proxy req logging and explicit upgrade handling"
sharon-wang Oct 29, 2024
d4fc66d
disable positron proxy for streamlit on PWB
sharon-wang Oct 29, 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
Prev Previous commit
Next Next commit
WIP: extra handling in positron proxy for websockets
  • Loading branch information
sharon-wang committed Oct 29, 2024
commit b91c7f5f6ed2099a6226e1122fbec75c1d212345
30 changes: 28 additions & 2 deletions extensions/positron-proxy/src/positronProxy.ts
Original file line number Diff line number Diff line change
@@ -454,7 +454,7 @@ export class PositronProxy implements Disposable {
target: targetOrigin,
changeOrigin: true,
selfHandleResponse: true,
ws: true,
ws: false,
onProxyReq: (proxyReq, req, _res, _options) => {
log.trace(`onProxyReq - proxy request ${serverOrigin}${req.url} -> ${targetOrigin}${req.url}` +
`\n\tmethod: ${proxyReq.method}` +
@@ -516,9 +516,35 @@ export class PositronProxy implements Disposable {
// Add the proxy middleware.
app.use('*', requestHandler);

// Is this happening? https://github.com/chimurai/http-proxy-middleware/issues/463

// Is this upgrade handling missing? https://github.com/chimurai/http-proxy-middleware?tab=readme-ov-file#websocket
// Handle the upgrade event.
server.on('upgrade', requestHandler.upgrade!);
// server.on('upgrade', requestHandler.upgrade!);

// For Positron Desktop or Server Web to work, we need to either:
// - set ws: true in the createProxyMiddleware options; OR
// - don't include `ws` and instead call server.on('upgrade', requestHandler.upgrade!)
// If we don't do one of these, the WebSocket connection will fail.

// For Positron on Workbench:
// - setting (true OR false) or not setting ws doesn't work
// - calling server.on('upgrade', requestHandler.upgrade!) doesn't work
// Doing either or even both of these doesn't work.

// Possible solution:
// - set ws: false in the createProxyMiddleware options
// - call server.on('upgrade', <CUSTOM_HANDLER>) where <CUSTOM_HANDLER> is a function that
// calls requestHandler.upgrade! if we're not running in Workbench, but sends the request
// to the Workbench proxy server if we are.
server.on('upgrade', (req, socket, head) => {
log.trace(`upgrade event for ${serverOrigin}${req.url}`);
if (!process.env.RS_SERVER_URL) {
requestHandler.upgrade!(req, socket, head);
} else {
// Send the request to the Workbench proxy server?
}
});
}

//#endregion Private Methods