Skip to content

Fix: iframe and sandbox retry mechanism#2986

Merged
Kitenite merged 8 commits intomainfrom
min
Oct 11, 2025
Merged

Fix: iframe and sandbox retry mechanism#2986
Kitenite merged 8 commits intomainfrom
min

Conversation

@saddlepaddle
Copy link
Copy Markdown
Contributor

@saddlepaddle saddlepaddle commented Oct 9, 2025

Description

  1. Have the script injection wait for the sandbox provider to be ready
  2. Add extra retry logic for starting up a sandbox (we were still failing even with the server-side retries)
  3. Allows the sandbox to still be accessible if the preload script injection fails (probably better than fully bricking the product)
  4. (main issue) Prevent SessionManager from starting before sandboxmanager is ready

Related Issues

Type of Change

  • Bug fix
  • New feature
  • Documentation
  • Refactor
  • Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes

Summary by CodeRabbit

  • Bug Fixes

    • Clear failure feedback when the project frame can’t connect, avoiding silent errors.
    • More accurate preload readiness detection to reduce stuck loading overlays.
    • Automatic retry for sandbox connections to minimize transient connection failures.
    • Multi-branch restore: removed misleading success toast on partial success; shows an error if any restore fails.
  • Refactor

    • Streamlined internal state management for preload and connection flow to improve reliability.

@vercel vercel bot temporarily deployed to Preview – docs October 9, 2025 21:03 Inactive
@vercel
Copy link
Copy Markdown

vercel bot commented Oct 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
web Ready Ready Preview Comment Oct 11, 2025 1:58am
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Oct 11, 2025 1:58am

@supabase
Copy link
Copy Markdown

supabase bot commented Oct 9, 2025

This pull request has been ignored for the connected project wowaemfasoptxrdjhilu because there are no changes detected in apps/backend/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 9, 2025

Caution

Review failed

The pull request is closed.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description lists the main changes clearly but leaves the Related Issues section without any linked issue references, does not select the appropriate Type of Change checkbox, and lacks a Testing section describing verification steps. These missing elements are required by the repository template to trace issue context, identify the nature of the change, and validate functionality. Because these sections remain as placeholders, the description is incomplete. Filling in these sections will improve PR clarity and adherence to project standards. Please link any related issues in the Related Issues section using GitHub keywords, mark the appropriate box under Type of Change to classify this PR, and provide details of the tests performed or steps to verify the changes in the Testing section.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly captures the main change of enforcing sequential sandbox loading to prevent lockouts, matching the pull request objectives. It is concise, specific, and describes the key fix without extraneous details. The phrasing aligns with the code modifications to session startup and script injection. Therefore it accurately summarizes the primary change.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 256d36b and 672bdde.

📒 Files selected for processing (2)
  • apps/web/client/src/app/project/[id]/_components/canvas/frame/index.tsx (2 hunks)
  • apps/web/client/src/components/store/editor/sandbox/index.ts (5 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@saddlepaddle saddlepaddle force-pushed the fix-git-restore-breaking branch from 6b7446b to e944991 Compare October 10, 2025 01:09
@vercel vercel bot temporarily deployed to Preview – docs October 10, 2025 01:18 Inactive
Base automatically changed from fix-git-restore-breaking to main October 10, 2025 02:58
@vercel vercel bot temporarily deployed to Preview – docs October 10, 2025 03:40 Inactive
console.error('[SandboxManager] Failed to ensure preload script exists:', error);
// Mark as injected to prevent blocking frames indefinitely
// Frames will handle the missing preload script gracefully
this.preloadScriptInjected = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we do this in finally instead of doing both in try and catch?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in this case we should just remove this flag or merge it with the preloadScriptLoading flag

@vercel vercel bot temporarily deployed to Preview – docs October 11, 2025 01:55 Inactive
@Kitenite Kitenite merged commit cea4916 into main Oct 11, 2025
4 of 7 checks passed
@Kitenite Kitenite changed the title Fix: ensure that sandbox loading steps occur sequentially to avoid locking out sandboxes Fix: iframe and sandbox retry mechanism Oct 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants