temporalio / sdk-go

Temporal Go SDK
https://docs.temporal.io/application-development?lang=go
MIT License
523 stars 208 forks source link

Starting DevServer with UI Enabled May Fail Due to Invalid Port Number #1526

Open tomwheeler opened 3 months ago

tomwheeler commented 3 months ago

Expected Behavior

I would expect that if I start the Dev Server, a valid functional test would pass regardless of how the DevServerOptions' EnableUI attribute was set.

Actual Behavior

A functional test written by a co-worker started the Dev Server, setting EnableUI to true. I found that this caused a failure in at least one case because the SDK calculated an invalid (out of range) port number.

Steps to Reproduce the Problem

I cannot reliably reproduce the problem and would imagine that it happens quite infrequently, but here's the output from a time when it failed:

go test -cover ./app/test -coverpkg ./... -args -test.gocoverdir=D:/a/tora-for-ci-testing/tora-for-ci-testing/.coverage/functional
2024/06/20 19:57:34 INFO  Downloading temporal CLI Url https://temporal.download/assets/temporalio/cli/releases/download/v0.13.1/temporal_cli_0.13.1_windows_amd64.zip ExePath C:\Users\RUNNER~1\AppData\Local\Temp\temporal-cli-go-sdk-1.26.0.exe
2024/06/20 19:57:36 INFO  Starting DevServer ExePath C:\Users\RUNNER~1\AppData\Local\Temp\temporal-cli-go-sdk-1.26.0.exe Args [server start-dev --ip 127.0.0.1 --port 65146 --namespace default --dynamic-config-value frontend.enableServerVersionCheck=false --dynamic-config-value system.forceSearchAttributesCacheRefreshOnRead=true]
time=2024-06-20T19:57:36.436 level=ERROR msg="can't use default UI port 66146 (65146 + 1000): listen tcp: address 66146: invalid port"
--- FAIL: Test_Order (67.38s)
    order_test.go:81: 
            Error Trace:    D:/a/tora-for-ci-testing/tora-for-ci-testing/app/test/order_test.go:81
            Error:          Received unexpected error:
                            failed connecting after timeout, last error: failed reaching server: last connection error: connection error: desc = "transport: Error while dialing: dial tcp 127.0.0.1:65146: connectex: No connection could be made because the target machine actively refused it."

This occurred when testing on Windows, although I doubt that it's limited to that platform. The heart of the problem seems to be the "listen tcp: address 66146: invalid port" message; the part preceding it suggests that it's calculating a port number without considering whether the result exceeds the allowed range.

Specifications

cretz commented 3 months ago

I think the solution here is to have a UIPort option, and its default should be random if server port is random, +1000 if server port is present, or allow it to be set explicitly.