-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix: suppress expected "No such container" error when removing build container #7070
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
base: next
Are you sure you want to change the base?
Conversation
v4.0.0-beta.438
…container During deployment cleanup, the build container is stopped and then removed. The removal command often fails with "No such container" because the container was already auto-removed when stopped. While the error is properly ignored by the deployment logic, it still appears in logs, creating unnecessary noise. This change redirects stderr to `/dev/null` and adds ` || true` to ensure the command always succeeds, suppressing the expected error message.
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/Jobs/ApplicationDeploymentJob.php (1)
2989-3000: I'll be back... but your container won't! Good error suppression, but could be more surgical.The addition of
2>/dev/null || trueon line 2995 makes sense for idempotent cleanup operations - like a good self-hosted server that just keeps running no matter what (unlike those flaky serverless functions that disappear faster than my gluten-free taco shells).However, you're suppressing all stderr here, including potentially important errors like:
- Docker daemon connection issues
- Permission problems
- Disk space issues
Consider a more targeted approach:
- ["docker rm -f $containerName 2>/dev/null || true", 'hidden' => true, 'ignore_errors' => true] + ["docker rm -f $containerName 2>&1 | grep -v 'No such container' || true", 'hidden' => true, 'ignore_errors' => true]Or even better, check if the container exists first:
+ ["docker ps -aq -f name=^${containerName}$ | xargs -r docker rm -f", 'hidden' => true, 'ignore_errors' => true]This way, you only remove containers that actually exist, and legitimate errors still surface for debugging. Your deployment logs will thank you later when troubleshooting real issues.
Alternative: If you want to keep the current approach (which is simpler), at least add a comment explaining why we're suppressing all errors:
// Suppress "No such container" errors - container may already be removed ["docker rm -f $containerName 2>/dev/null || true", 'hidden' => true, 'ignore_errors' => true]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Jobs/ApplicationDeploymentJob.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
app/Jobs/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Queueable background work should be implemented as Jobs under app/Jobs; Actions may expose asJob to dispatch
app/Jobs/**/*.php: Queue jobs should implement ShouldQueue and define retry strategy (tries, maxExceptions, backoff)
Queue jobs must handle success/failure state updates and broadcast corresponding events; implement failed(Throwable) for final failure handlingQueue jobs should update status, wrap external calls in try/catch, set failure status on exceptions, and rethrow
Files:
app/Jobs/ApplicationDeploymentJob.php
app/Jobs/*.php
📄 CodeRabbit inference engine (.cursor/rules/deployment-architecture.mdc)
Implement background deployment, monitoring, backup, and notification work as Laravel Job classes under app/Jobs
Files:
app/Jobs/ApplicationDeploymentJob.php
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
**/*.php: Follow PSR-12 coding standards for all PHP code
Format PHP code with Laravel Pint configuration
Run static analysis with PHPStan to ensure type safety in PHP code
Document complex methods with PHPDoc blocks including parameters, return types, and thrown exceptions
**/*.php: Follow PSR-12 PHP coding standards across the codebase
Prefer eager loading and query optimization to prevent N+1 issues in database interactions
Use Laravel best practices for structure, services, and policies
**/*.php: Always use curly braces for control structures in PHP, even for single-line bodies
Use PHP 8 constructor property promotion in __construct() and avoid empty constructors
Always declare explicit return types for functions and methods; add appropriate parameter type hints
Prefer PHPDoc blocks over inline comments; only add inline code comments for very complex logic
When documenting arrays, add useful array shape types in PHPDoc where appropriate
Enum case names should be TitleCase (e.g., FavoritePerson, Monthly)
Files:
app/Jobs/ApplicationDeploymentJob.php
app/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
Use database transactions to group related write operations for consistency in services/controllers/jobs
Files:
app/Jobs/ApplicationDeploymentJob.php
app/Jobs/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Implement background jobs under app/Jobs
Files:
app/Jobs/ApplicationDeploymentJob.php
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/deployment-architecture.mdc:0-0
Timestamp: 2025-08-27T15:01:10.040Z
Learning: Applies to app/Jobs/*.php : Implement background deployment, monitoring, backup, and notification work as Laravel Job classes under app/Jobs
📚 Learning: 2025-08-27T15:01:10.040Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/deployment-architecture.mdc:0-0
Timestamp: 2025-08-27T15:01:10.040Z
Learning: Applies to app/Jobs/*.php : Implement background deployment, monitoring, backup, and notification work as Laravel Job classes under app/Jobs
Applied to files:
app/Jobs/ApplicationDeploymentJob.php
Changes
private function graceful_shutdown_container()'ignore_errors' => trueSummary by CodeRabbit
Release Notes