8000 Fix issues with webscockets by ktaletsk · Pull Request #1623 · mito-ds/mito · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix issues with webscockets #1623

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Apr 15, 2025
Merged

Fix issues with webscockets #1623

merged 12 commits into from
Apr 15, 2025

Conversation

ktaletsk
Copy link
Collaborator
@ktaletsk ktaletsk commented Apr 11, 2025 •
edited
Loading

Description

Replace HEAD request with query param on the main GET request

In situation where JupyterLab is running behind OAuth 2, like in JupyterHub with HTTPS enabled, the HEAD method is not supported. We use HEAD request to check the connection before opening a websocket. This PR replaces HEAD request with a query parameter in the main GET request thus enabling the extension to work with OAuth 2 flow.

Fix websocket reconnection issues

  1. Added error handling to the initialize() method to better report connection failures
  2. Implemented a flag _initializationInProgress to prevent multiple concurrent initialization attempts
  3. Added proper cleanup of existing sockets before creating new ones
  4. Added a connection timeout to detect stalled connections
  5. Reset reconnection counters after failures to ensure future attempts can succeed
  6. Created a more robust error propagation

@ktaletsk ktaletsk requested review from aarondr77 and Copilot April 11, 2025 23:10
@vercel Vercel
Copy link
vercel bot commented Apr 11, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
monorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 14, 2025 10:50pm

Copilot
Copilot AI reviewed Apr 11, 2025
View reviewed changes
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

@ktaletsk @Copilot
Update mito-ai/mito_ai/handlers.py
1b6672a 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
aarondr77
aarondr77 requested changes Apr 14, 2025
View reviewed changes
@ktaletsk ktaletsk requested review from aarondr77 and Copilot April 14, 2025 02:46
Copilot
Copilot AI reviewed Apr 14, 2025
View reviewed changes
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

@ktaletsk
fix websocket client initialization issues
b2026df 
added error handling during initialization
implemented a mechanism to prevent multiple concurrent initializations
introduced a connection timeout for websocket connections
improved error propagation for service availability checks and reconnection attempts
@ktaletsk ktaletsk self-assigned this Apr 15, 2025
@ktaletsk ktaletsk changed the title Replace HEAD request with query param on the main GET request Fix issues with webscockets Apr 15, 2025
@aarondr77 aarondr77 merged commit 3a0dab8 into dev Apr 15, 2025
3 checks passed
@ktaletsk ktaletsk deleted the fix-remove-head-request branch April 16, 2025 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@aarondr77 aarondr77 Awaiting requested review from aarondr77

Assignees

@ktaletsk ktaletsk

Labels
project: mito-ai
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ktaletsk @aarondr77
0