-
Notifications
You must be signed in to change notification settings - Fork 486
fix(cli): stop declarative sync hanging on edge-runtime container exit #5714
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+307
−5
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
d4e2d74
fix(cli): force-close Edge Runtime worker after pg-delta scripts emit…
claude 8e4b4bb
Merge branch 'develop' into claude/issue-312-investigation-mec6f6
avallete 7908b63
fix(cli): force-close worker in pgcache catalog export too
claude e96bb3f
Merge branch 'develop' into claude/issue-312-investigation-mec6f6
avallete 1d4b65b
Merge branch 'develop' into claude/issue-312-investigation-mec6f6
avallete 87edeed
fix(cli): wait for edge-runtime container exit instead of following i…
claude 4d69470
Merge branch 'develop' into claude/issue-312-investigation-mec6f6
avallete 35e2554
Merge branch 'develop' into claude/issue-312-investigation-mec6f6
avallete adb0910
Merge branch 'develop' into claude/issue-312-investigation-mec6f6
avallete File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package diff | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // lastCodeLine returns the final non-blank, non-comment line of a script. | ||
| func lastCodeLine(script string) string { | ||
| lines := strings.Split(script, "\n") | ||
| for i := len(lines) - 1; i >= 0; i-- { | ||
| line := strings.TrimSpace(lines[i]) | ||
| if line == "" || strings.HasPrefix(line, "//") { | ||
| continue | ||
| } | ||
| return line | ||
| } | ||
| return "" | ||
| } | ||
|
|
||
| // Every pg-delta edge-runtime script must force the worker's event loop closed | ||
| // once its output has been written. The pg connection pool can leave keepalive | ||
| // handles registered even after close() resolves; if the worker never exits, | ||
| // the container never stops and the CLI — which streams the container logs with | ||
| // Follow:true — blocks forever following them, hanging declarative sync at 0% | ||
| // CPU (supabase/pg-toolbelt#312). The success path must terminate | ||
| // unconditionally rather than rely on the event loop draining on its own, so | ||
| // guard against the force-close being dropped from any template's success path. | ||
| func TestPgDeltaScriptsForceCloseOnSuccess(t *testing.T) { | ||
| scripts := map[string]string{ | ||
| "pgdelta.ts": pgDeltaScript, | ||
| "pgdelta_declarative_export.ts": pgDeltaDeclarativeExportScript, | ||
| "pgdelta_catalog_export.ts": pgDeltaCatalogExportScript, | ||
| } | ||
| for name, script := range scripts { | ||
| t.Run(name, func(t *testing.T) { | ||
| require.NotEmpty(t, script) | ||
| // The terminating statement runs on the success path (the catch | ||
| // branch no longer re-throws), so the worker is torn down whether | ||
| // or not the body succeeded. | ||
| assert.Equal(t, `throw new Error("");`, lastCodeLine(script), | ||
| "success path must force the Edge Runtime worker to exit so the container stops") | ||
| }) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| package pgcache | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // The migrations-catalog cache script (db start / db push with pg-delta caching) | ||
| // opens a connection pool and must force the worker's event loop closed once it | ||
| // has written its snapshot. If a keepalive handle lingers after close() resolves | ||
| // the worker never exits, so the container never stops and the CLI — which | ||
| // follows the container logs with Follow:true — hangs indefinitely at 0% CPU | ||
| // (supabase/pg-toolbelt#312). Guard against the success-path force-close being | ||
| // dropped. | ||
| func TestPgDeltaCatalogExportScriptForceClosesOnSuccess(t *testing.T) { | ||
| require.NotEmpty(t, pgDeltaCatalogExportTS) | ||
|
|
||
| lines := strings.Split(pgDeltaCatalogExportTS, "\n") | ||
| last := "" | ||
| for i := len(lines) - 1; i >= 0; i-- { | ||
| line := strings.TrimSpace(lines[i]) | ||
| if line == "" || strings.HasPrefix(line, "//") { | ||
| continue | ||
| } | ||
| last = line | ||
| break | ||
| } | ||
| assert.Equal(t, `throw new Error("");`, last, | ||
| "success path must force the Edge Runtime worker to exit so the container stops") | ||
| } |
33 changes: 33 additions & 0 deletions
33
apps/cli-go/internal/pgdelta/pgdelta_apply_template_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| package pgdelta | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // The declarative-apply script connects to TARGET and must force the worker's | ||
| // event loop closed once it has written its result JSON. applyDeclarativeSchema | ||
| // can leave connection keepalive handles registered, and if the worker never | ||
| // exits the container never stops — the CLI, which follows the container logs | ||
| // with Follow:true, then hangs indefinitely at 0% CPU (supabase/pg-toolbelt#312). | ||
| // The success path must terminate unconditionally, so guard against the | ||
| // force-close being dropped. | ||
| func TestDeclarativeApplyScriptForceClosesOnSuccess(t *testing.T) { | ||
| require.NotEmpty(t, pgDeltaDeclarativeApplyScript) | ||
|
|
||
| lines := strings.Split(pgDeltaDeclarativeApplyScript, "\n") | ||
| last := "" | ||
| for i := len(lines) - 1; i >= 0; i-- { | ||
| line := strings.TrimSpace(lines[i]) | ||
| if line == "" || strings.HasPrefix(line, "//") { | ||
| continue | ||
| } | ||
| last = line | ||
| break | ||
| } | ||
| assert.Equal(t, `throw new Error("");`, last, | ||
| "success path must force the Edge Runtime worker to exit so the container stops") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package utils | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/docker/docker/api/types/container" | ||
| "github.com/docker/docker/api/types/network" | ||
| "github.com/h2non/gock" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| "github.com/supabase/cli/internal/testing/apitest" | ||
| ) | ||
|
|
||
| // DockerRunOnceWaitWithConfig must detect container completion via inspect and | ||
| // read logs without following the stream. Following the log stream to detect | ||
| // exit hangs forever under podman, whose /logs?follow endpoint never closes when | ||
| // the container stops (supabase/pg-toolbelt#312). These tests pin that the runner | ||
| // reads the captured output and maps the inspected exit code to an error. | ||
| func TestDockerRunOnceWait(t *testing.T) { | ||
| imageUrl := GetRegistryImageUrl(imageId) | ||
|
|
||
| t.Run("waits for exit then reads buffered logs", func(t *testing.T) { | ||
| require.NoError(t, apitest.MockDocker(Docker)) | ||
| defer gock.OffAll() | ||
| apitest.MockDockerStart(Docker, imageUrl, containerId) | ||
| require.NoError(t, apitest.MockDockerLogs(Docker, containerId, "CATALOG\n")) | ||
| // Run test | ||
| var stdout, stderr bytes.Buffer | ||
| err := DockerRunOnceWaitWithConfig( | ||
| context.Background(), | ||
| container.Config{Image: imageUrl}, | ||
| container.HostConfig{}, | ||
| network.NetworkingConfig{}, | ||
| containerId, | ||
| &stdout, | ||
| &stderr, | ||
| ) | ||
| // Validate | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, "CATALOG\n", stdout.String()) | ||
| assert.Empty(t, apitest.ListUnmatchedRequests()) | ||
| }) | ||
|
|
||
| t.Run("maps non-zero exit code to error", func(t *testing.T) { | ||
| require.NoError(t, apitest.MockDocker(Docker)) | ||
| defer gock.OffAll() | ||
| apitest.MockDockerStart(Docker, imageUrl, containerId) | ||
| require.NoError(t, apitest.MockDockerLogsExitCode(Docker, containerId, 1)) | ||
| // Run test | ||
| var stdout, stderr bytes.Buffer | ||
| err := DockerRunOnceWaitWithConfig( | ||
| context.Background(), | ||
| container.Config{Image: imageUrl}, | ||
| container.HostConfig{}, | ||
| network.NetworkingConfig{}, | ||
| containerId, | ||
| &stdout, | ||
| &stderr, | ||
| ) | ||
| // Validate | ||
| assert.ErrorContains(t, err, "error running container: exit 1") | ||
| assert.Empty(t, apitest.ListUnmatchedRequests()) | ||
| }) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.