Skip to content

WIP: Add wvpc tcp service and hyperdrive -> wvpc support#12561

Open
xortive wants to merge 1 commit intocloudflare:mainfrom
xortive:malonso/hyperdrive-wvpc-wip
Open

WIP: Add wvpc tcp service and hyperdrive -> wvpc support#12561
xortive wants to merge 1 commit intocloudflare:mainfrom
xortive:malonso/hyperdrive-wvpc-wip

Conversation

@xortive
Copy link
Contributor

@xortive xortive commented Feb 13, 2026

@xortive xortive requested a review from a team as a code owner February 13, 2026 23:54
@changeset-bot
Copy link

changeset-bot bot commented Feb 13, 2026

⚠️ No Changeset found

Latest commit: 7403a39

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

View 5 additional findings in Devin Review.

Open in Devin Review

// Display service-specific details
if (service.type === ServiceType.Http) {
if (service.type === ServiceType.Tcp) {
logger.log(` TCP Port: ${service.tcp_port}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 Missing undefined guard for tcp_port causes "TCP Port: undefined" output

When creating a TCP VPC service, service.tcp_port is logged unconditionally, but tcp_port is an optional field (tcp_port?: number in the ConnectivityService interface at packages/wrangler/src/vpc/index.ts:65). If the API returns a TCP service without a tcp_port value, the output will display TCP Port: undefined to the user.

Inconsistency with HTTP port handling

The HTTP port handling on lines 57-63 correctly guards against undefined values:

if (service.http_port) {
    logger.log(`   HTTP Port: ${service.http_port}`);
}
if (service.https_port) {
    logger.log(`   HTTPS Port: ${service.https_port}`);
}

But the TCP branch at line 56 logs unconditionally:

logger.log(`   TCP Port: ${service.tcp_port}`);

Since tcp_port is optional and the --tcp-port CLI option is not required (demandOption is not set, see packages/wrangler/src/vpc/index.ts:102-107), the API could return a response without tcp_port, resulting in the string "TCP Port: undefined" being printed to the user.

Suggested change
logger.log(` TCP Port: ${service.tcp_port}`);
if (service.tcp_port) {
logger.log(` TCP Port: ${service.tcp_port}`);
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

// Display service-specific details
if (service.type === ServiceType.Http) {
if (service.type === ServiceType.Tcp) {
logger.log(` TCP Port: ${service.tcp_port}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 Missing undefined guard for tcp_port causes "TCP Port: undefined" output in update handler

Same issue as in create.ts: the update.ts handler logs service.tcp_port unconditionally at line 61, but tcp_port is optional. This will display TCP Port: undefined if the field is not set.

Inconsistency with HTTP port handling

The HTTP port handling on lines 62-68 correctly guards against undefined values with if (service.http_port) and if (service.https_port), but the TCP branch at line 61 does not:

logger.log(`   TCP Port: ${service.tcp_port}`);

This inconsistency means TCP services could show malformed output to the user.

Suggested change
logger.log(` TCP Port: ${service.tcp_port}`);
if (service.tcp_port) {
logger.log(` TCP Port: ${service.tcp_port}`);
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 14, 2026

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@12561

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@12561

miniflare

npm i https://pkg.pr.new/miniflare@12561

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@12561

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@12561

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@12561

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@12561

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@12561

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@12561

wrangler

npm i https://pkg.pr.new/wrangler@12561

commit: 7403a39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

1 participant

Comments