Skip to content

fix(deploy): notify on container crash loop after deployment#72

Open
AminDhouib wants to merge 3 commits into
canaryfrom
port/upr-4357
Open

fix(deploy): notify on container crash loop after deployment#72
AminDhouib wants to merge 3 commits into
canaryfrom
port/upr-4357

Conversation

@AminDhouib

@AminDhouib AminDhouib commented Jul 12, 2026

Copy link
Copy Markdown
Member

Ports Dokploy#4357 by @sajdakabir (1 commit, cherry-picked with -x, original authorship preserved) + 1 fork-side test-adaptation commit.

Addresses Dokploy#4339 (verified: linked via the upstream PR's closing references; the diff makes a post-deploy crash loop surface as a failed deployment + build-error notification instead of a silent "done").

What / why

Previously a deployment was marked done (and success notifications sent) as soon as the container was (re)created — if the app then crash-looped, nothing was reported.

  • New waitForSwarmServiceStable(appName, { serverId, windowMs = 60s, pollMs = 5s }) in packages/server/src/utils/docker/utils.ts: polls the swarm service's tasks; latest task failed/rejected, or a restart after having been running, → unstable (with reason).
  • deployApplication / rebuildApplication run this check after mechanizeDockerContainer; instability throws, which routes into the existing error path: deployment/application marked error and sendBuildErrorNotifications fires (existing payloads only — no DB schema change, no migration).

Reviewer heads-up: by upstream design, every successful application deploy now includes a fixed 60s stability window before being marked done.

Adaptations (fork-side, test-only)

Upstream PR Dokploy#4357's own pr-check (test) is red; reproduced locally — the stability poll hits a real Docker socket from unit tests:

  1. __test__/deploy/application.command.test.ts: partial mock of docker/utils stubbing waitForSwarmServiceStable{ stable: true } (rest of the module stays real). Without it, 4 command-generation tests time out (5s limit vs 60s poll) — exactly upstream's CI failure.
  2. __test__/deploy/application.real.test.ts: REAL_TEST_TIMEOUT 180s → 300s, since each successful real deploy now includes the fixed 60s window on top of real nixpacks/railpack build time.

No production-code deviation from upstream.

Test evidence (Windows local; Linux CI is source of truth)

  • __test__/deploy/ suite: 52 passed / 1 skipped; application.command.test.ts 6/6 (the 4 pre-adaptation timeouts are gone). Only application.real.test.ts fails locally — in beforeEach shell setup, pre-existing Windows-local behavior for this Linux-only file, unrelated to this change.
  • pnpm --filter=@dokploy/server run typecheck and pnpm --filter=dokploy run typecheck: clean.

Sibling / merge-order notes

sajdakabir and others added 2 commits July 12, 2026 10:48
- application.command.test.ts: stub waitForSwarmServiceStable (partial mock,
  rest of docker/utils stays real) — command-generation tests have no real
  swarm service to poll, so the new 60s stability window timed them out.
- application.real.test.ts: raise REAL_TEST_TIMEOUT 180s -> 300s to absorb
  the fixed 60s stability window each successful real deploy now includes.

Fork-side adaptation for the port of Dokploy#4357 (upstream's own
pr-check(test) was red for exactly this reason).
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.

2 participants