fix(probe): use protocol-aware default port for endpoint probing

[John-Lin]

fix #3

Previously the probe always fell back to port 4317 when no explicit port was provided, even for http:// (80) and https:// (443) URLs. This caused the probe to check a different port than the OTel SDK would actually connect to. Extract parseEndpoint to make the parsing logic independently testable.

See also https://opentelemetry.io/docs/specs/otel/protocol/exporter/#endpoint-urls-for-otlphttp

That also means, if the port is empty or not given, TCP port 80 is the default for the http scheme and TCP port 443 is the default for the https scheme,

Summary by CodeRabbit

• New Features

  • Added endpoint URL parsing capability with automatic port assignment based on protocol scheme: HTTP defaults to port 80, HTTPS to port 443, and other schemes to port 4317.

• Refactor

  • Improved endpoint validation logic with enhanced error handling and clearer feedback for invalid or malformed URLs.

[coderabbitai]

📝 Walkthrough

Walkthrough

A new parseEndpoint helper function was added to parse endpoint URLs and apply scheme-based default ports (80 for http, 443 for https, 4317 for unknown protocols). The existing probeEndpoint function was refactored to use this helper, fixing port detection for URLs without explicit ports. Comprehensive tests were added for the new function.

Changes

Cohort / File(s)	Summary
Endpoint Parsing Logic src/probe.ts	Added parseEndpoint function to extract and centralize URL parsing with scheme-aware default port assignment. Refactored probeEndpoint to use this function, replacing inline port resolution logic and fixing the issue where http/https endpoints without explicit ports defaulted to 4317.
Test Coverage tests/probe.test.ts	Added comprehensive test suite for parseEndpoint covering http/https with implicit and explicit ports, unknown protocols, and invalid URLs. Existing probeEndpoint tests remain intact.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

Ports were muddled, confused in the night,
443 hiding while 4317 shined bright,
A helper extracted, logic made clear,
Now HTTPS endpoints find their path here! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: fixing endpoint probing to use protocol-aware default ports (80 for http, 443 for https) instead of always falling back to 4317.
Linked Issues check	✅ Passed	The PR directly addresses issue #3 by implementing protocol-aware default ports (80 for http, 443 for https) in the new parseEndpoint function and refactored probeEndpoint to use it.
Out of Scope Changes check	✅ Passed	All changes are in-scope: the new parseEndpoint function directly solves the issue, the refactored probeEndpoint uses it correctly, and test coverage is added for the new functionality.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

📝 Coding Plan for PR comments

• Generate coding plan

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/probe.ts (1)

6-18: ⚠️ Potential issue | 🟡 Minor

JSDoc comment is misplaced.

The JSDoc at lines 6-9 describes probeEndpoint ("Opens a TCP connection...") but is positioned directly above parseEndpoint. Move this comment to directly precede probeEndpoint, and consider adding a separate JSDoc for parseEndpoint.

📝 Proposed fix

-/** Result of a TCP connectivity probe against the OTLP endpoint. */
-export type ProbeResult = { ok: boolean; ms: number; error?: string }
-
-/**
- * Opens a TCP connection to the host and port parsed from `endpoint` to verify
- * reachability before the OTel SDK initialises. Resolves within 5 seconds.
- */
-export function parseEndpoint(endpoint: string): { host: string; port: number } | null {
+/** Result of a TCP connectivity probe against the OTLP endpoint. */
+export type ProbeResult = { ok: boolean; ms: number; error?: string }
+
+/** Parses an endpoint URL and returns the host and port, applying protocol-aware defaults. */
+export function parseEndpoint(endpoint: string): { host: string; port: number } | null {
   try {
     const url = new URL(endpoint)
     const defaultPort = url.protocol === "http:" ? 80 : url.protocol === "https:" ? 443 : 4317
     return { host: url.hostname, port: url.port ? parseInt(url.port, 10) : defaultPort }
   } catch {
     return null
   }
 }
+
+/**
+ * Opens a TCP connection to the host and port parsed from `endpoint` to verify
+ * reachability before the OTel SDK initialises. Resolves within 5 seconds.
+ */
+export function probeEndpoint(endpoint: string): Promise<ProbeResult> {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/probe.ts` around lines 6 - 18, Move the misplaced JSDoc that currently
precedes parseEndpoint so it directly precedes probeEndpoint (the comment about
opening a TCP connection and 5s timeout belongs to probeEndpoint), and add a
short JSDoc above parseEndpoint describing its purpose and return value (parses
an endpoint string into host and numeric port or returns null). Ensure the
comment text matches the functions: keep the TCP/reachability text with
probeEndpoint and a concise parse description for parseEndpoint.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/probe.ts`:
- Around line 6-18: Move the misplaced JSDoc that currently precedes
parseEndpoint so it directly precedes probeEndpoint (the comment about opening a
TCP connection and 5s timeout belongs to probeEndpoint), and add a short JSDoc
above parseEndpoint describing its purpose and return value (parses an endpoint
string into host and numeric port or returns null). Ensure the comment text
matches the functions: keep the TCP/reachability text with probeEndpoint and a
concise parse description for parseEndpoint.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 63d1f8fa-4119-46af-82e4-46f68475640b

📥 Commits

Reviewing files that changed from the base of the PR and between f0af602 and 73ae4d0.

📒 Files selected for processing (2)

• src/probe.ts
• tests/probe.test.ts