-
Notifications
You must be signed in to change notification settings - Fork 17
feat(dev): Wait for API server to start before sending gql requests #1015
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: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cedarjs canceled.
|
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many -t build:pack --exclude create-ceda... |
✅ Succeeded | 2s | View ↗ |
nx run-many -t build |
✅ Succeeded | 5s | View ↗ |
nx run-many -t test --minWorkers=1 --maxWorkers=4 |
✅ Succeeded | 1m 45s | View ↗ |
nx run-many -t test:types |
✅ Succeeded | 10s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-16 18:47:04 UTC
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many -t build |
✅ Succeeded | 5s | View ↗ |
nx run-many -t test:types |
✅ Succeeded | 10s | View ↗ |
nx run-many -t build:pack --exclude create-ceda... |
✅ Succeeded | <1s | View ↗ |
nx run-many -t test --minWorkers=1 --maxWorkers=4 |
✅ Succeeded | 1m 25s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-16 18:34:15 UTC
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many -t build |
✅ Succeeded | 5s | View ↗ |
nx run-many -t build:pack --exclude create-ceda... |
✅ Succeeded | 4s | View ↗ |
nx run-many -t test --minWorkers=1 --maxWorkers=4 |
✅ Succeeded | 1m 45s | View ↗ |
nx run-many -t test:types |
✅ Succeeded | 10s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-16 18:35:09 UTC
Greptile SummaryAdded a Vite plugin middleware that waits for the API server to be available before proxying GraphQL and API requests, replacing the previous approach of simply suppressing ECONNREFUSED errors for 2.5 seconds with active port polling. Key changes:
Issues found:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Browser
participant Vite as Vite Dev Server
participant Plugin as Wait-for-API Plugin
participant API as API Server
Client->>Vite: Request /graphql
Vite->>Plugin: Middleware intercepts request
alt API request detected
Plugin->>Plugin: Check if URL matches API patterns
loop Poll every 500ms (max 60s)
Plugin->>API: Attempt TCP connection
alt Connection successful
API-->>Plugin: Port is open
Plugin->>Vite: Call next()
Vite->>API: Proxy request
API-->>Vite: Response
Vite-->>Client: Response
else Connection failed
API-->>Plugin: Port not ready
Plugin->>Plugin: Wait 500ms and retry
end
end
alt Timeout after 60s
Plugin->>Plugin: Log error
Plugin->>Vite: Call next() anyway
Note over Plugin,Vite: BUG: Should return 503 instead
Vite->>API: Proxy request (fails)
API-->>Vite: ECONNREFUSED
Vite-->>Client: Error
end
else Non-API request
Plugin->>Vite: Call next() immediately
Vite-->>Client: Handle normally
end
|
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.
2 files reviewed, 2 comments
|
|
||
| let shouldWait = false | ||
|
|
||
| if (url.includes('/graphql')) { |
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.
style: this check may match non-API URLs that happen to contain /graphql. consider using url === '/graphql' or url.startsWith('/graphql') for more precise matching
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/vite/src/plugins/vite-plugin-cedar-wait-for-api-server.ts
Line: 27:27
Comment:
**style:** this check may match non-API URLs that happen to contain `/graphql`. consider using `url === '/graphql'` or `url.startsWith('/graphql')` for more precise matching
How can I resolve this? If you propose a fix, please make it concise.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.
Yeah, I'm not entirely happy with the logic here. I'll do some more testing before merging this PR

No more ECONNREFUSED on startup!