-
Notifications
You must be signed in to change notification settings - Fork 4.4k
If build has failed, show correct error page #9569
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
Conversation
WalkthroughThis pull request enhances error handling for build processes by adding a specific configuration for the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as General Controller
participant ExceptionHandler as AppwriteException
Client->>Controller: Initiate deployment request
Controller->>Controller: Check deployment status
alt Status is waiting/processing/building
Controller->>ExceptionHandler: Throw BUILD_NOT_READY exception
else Status is failed
Controller->>ExceptionHandler: Throw BUILD_FAILED exception
end
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
✨ Benchmark results
⚡ Benchmark Comparison
|
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)
tests/e2e/Services/Sites/SitesCustomServerTest.php (1)
2397-2432
: Good test addition, but there's a typo in the method name.The test effectively verifies that an appropriate error page is displayed when attempting to access a domain associated with a failed build. However, the method name should be corrected.
- public function testDomainForFailedDeloyment(): void + public function testDomainForFailedDeployment(): void
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/controllers/general.php
(1 hunks)tests/e2e/Services/Sites/SitesCustomServerTest.php
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/controllers/general.php
🧰 Additional context used
🧬 Code Definitions (1)
tests/e2e/Services/Sites/SitesCustomServerTest.php (4)
tests/e2e/Services/Sites/SitesBase.php (5)
setupSite
(20-33)setupSiteDomain
(329-347)createDeployment
(237-245)getDeployment
(171-179)cleanupSite
(69-78)tests/e2e/Client.php (2)
Client
(8-317)call
(167-277)tests/e2e/Services/Functions/FunctionsBase.php (2)
createDeployment
(224-232)getDeployment
(160-168)src/Executor/Executor.php (1)
call
(290-416)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
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)
tests/e2e/Services/Sites/SitesCustomServerTest.php (1)
2397-2432
: Method name has a typo, but implementation correctly tests failed build error handling.The test method successfully verifies that when a router attempts to access a domain associated with a failed build, the correct error message is displayed. However, there's a typo in the method name:
testDomainForFailedDeloyment
should betestDomainForFailedDeployment
.- public function testDomainForFailedDeloyment(): void + public function testDomainForFailedDeployment(): void
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/e2e/Services/Sites/SitesCustomServerTest.php
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Databases)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Projects)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: Benchmark
- GitHub Check: E2E General Test
- GitHub Check: Unit Test
- GitHub Check: scan
What does this PR do?
If router tries to access a domain of a failed build, show appropriate error message.
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist
Summary by CodeRabbit