Skip to content

Conversation

@tkislan
Copy link
Member

@tkislan tkislan commented Nov 6, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Port availability checks now correctly treat systems without IPv6 as IPv6-unsupported, reducing false "port unavailable" reports and improving reliability when starting local services.
    • Enhanced logging for clearer diagnostics when IPv6 checks fail.
  • Tests

    • Added unit tests covering IPv6-unavailable scenarios and related logging to ensure consistent behavior.

Signed-off-by: Tomas Kislan <tomas@kislan.sk>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

📝 Walkthrough

Walkthrough

The isPortAvailable function now wraps the IPv6 (::1) port check in its own try/catch. IPv4 (127.0.0.1) is checked first; if that fails, the function returns false. If IPv4 succeeds, the IPv6 check runs inside a dedicated try/catch: an EAFNOSUPPORT error is treated as “IPv6 unsupported” (a debug is logged) and the function returns true; any other error logs a warning and the function returns false. No public API or exported declarations changed.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant isPortAvailable
    participant Logger

    Caller->>isPortAvailable: request port availability
    isPortAvailable->>isPortAvailable: check 127.0.0.1 (IPv4)
    alt IPv4 bind fails
        isPortAvailable->>Caller: return false
    else IPv4 bind succeeds
        isPortAvailable->>isPortAvailable: check ::1 (IPv6) within its own try/catch
        alt IPv6 bind succeeds
            isPortAvailable->>Caller: return true
        else IPv6 throws EAFNOSUPPORT
            Note right of isPortAvailable `#e8f5e9`: IPv6 unsupported (EAFNOSUPPORT)
            isPortAvailable->>Logger: debug "IPv6 is not supported on this system"
            isPortAvailable->>Caller: return true
        else IPv6 throws other error
            Note right of isPortAvailable `#fff3e0`: Unexpected IPv6 error
            isPortAvailable->>Logger: warn "Failed to check IPv6 port availability"
            isPortAvailable->>Caller: return false
        end
    end
Loading

Possibly related PRs

Suggested reviewers

  • Artmann

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: handling disabled IPv6 support in the deepnote-server, which matches the core modification to isPortAvailable's IPv6 error handling.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 254d178 and fd0ee38.

📒 Files selected for processing (1)
  • src/kernels/deepnote/deepnoteServerStarter.node.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/kernels/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)

src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations

Files:

  • src/kernels/deepnote/deepnoteServerStarter.node.ts
🧬 Code graph analysis (1)
src/kernels/deepnote/deepnoteServerStarter.node.ts (2)
build/launchWebUtils.js (1)
  • port (20-24)
src/platform/logging/index.ts (1)
  • logger (35-48)

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

@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73%. Comparing base (985cd44) to head (fd0ee38).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #192   +/-   ##
=====================================
  Coverage     73%     73%           
=====================================
  Files        577     577           
  Lines      47626   47663   +37     
  Branches    5596    5597    +1     
=====================================
+ Hits       34902   34939   +37     
  Misses     10887   10887           
  Partials    1837    1837           
Files with missing lines Coverage Δ
src/kernels/deepnote/deepnoteServerStarter.node.ts 25% <100%> (+1%) ⬆️
...ernels/deepnote/deepnoteServerStarter.unit.test.ts 98% <100%> (+<1%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 985cd44 and bd5fbf4.

📒 Files selected for processing (1)
  • src/kernels/deepnote/deepnoteServerStarter.node.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/kernels/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)

src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations

Files:

  • src/kernels/deepnote/deepnoteServerStarter.node.ts
🧬 Code graph analysis (1)
src/kernels/deepnote/deepnoteServerStarter.node.ts (1)
src/platform/logging/index.ts (1)
  • logger (35-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build & Test

Signed-off-by: Tomas Kislan <tomas@kislan.sk>
coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 6, 2025
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 6, 2025
@saltenasl saltenasl marked this pull request as ready for review November 6, 2025 09:17
@saltenasl saltenasl requested a review from a team as a code owner November 6, 2025 09:17
@saltenasl saltenasl merged commit 766e90e into main Nov 6, 2025
13 checks passed
@saltenasl saltenasl deleted the tk/handle-disabled-ipv6-deepnote-server-starter branch November 6, 2025 09:18
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.

3 participants