-
Notifications
You must be signed in to change notification settings - Fork 0
Fix environment variables validation error during deployment #2
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
|
🧙 Sourcery is reviewing your pull request! Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Co-authored-by: larp0 <204380501+larp0@users.noreply.github.com>
|
@l r |
Overall Summary of PRThis PR improves the deployment CLI by better handling environment variable sync errors returned by the server in the body of a 200 response. Previously, only a success boolean on the upload result was checked, but this change adds an extra condition for Mermaid Diagrams1. Whole Repo High-Level Component Diagram (Simplified)flowchart TD
%% ------------ Nodes ------------
A["CLI Commands e.g. deploy.ts"]
B["API Client apiClient.js"]
C["Build System (buildWorker.js, buildImage.js)"]
D["Config Loader (config.js)"]
E["Utilities"]
F["Env Var Utilities (localEnvVars.js, dotEnv.js)"]
G["Logging Utilities (logs.js, cliOutput.js)"]
H["Git Utilities (gitMeta.js)"]
I["Session Management (session.js)"]
J["Temp Directory (tempDirectories.js)"]
K["External Tools (docker called via tinyexec)"]
L["Schema & Types (core/src/v3/schemas/api.ts)"]
%% ------------ Links ------------
A --> B
A --> C
A --> D
A --> E
A --> K
E --> F
E --> G
E --> H
E --> I
E --> J
B --> L
%% ------------ Styling -----------
classDef external fill:#f9f,stroke:#333,stroke-width:1px,color:#333
class K external
This diagram shows the CLI command as the main orchestrator, calling the API client for server interactions and triggering local build and deployment processes. Utilities support env var loading, logging, git metadata, and session handling. The schema/types package defines API request/response shapes for type safety and validation. 2. Diagram of What Changed in This PRflowchart TD
%% ---------- Nodes ----------
A["_deployCommand function"]
B["SyncEnvVarsWithServer call"]
C["UploadResult Response Handling"]
F["spinner stop success message"]
I["EnvironmentVariableResponseBody Schema"]
J["EnvironmentVariableResponseBody Tests"]
%% ---------- Old logic ----------
subgraph "Old logic"
D{"uploadResult.success?"}
C --> D
D -- false --> E["failDeploy with uploadResult.error"]
D -- true --> F
end
%% ---------- New logic ----------
subgraph "New logic"
G{"Does uploadResult.data contain error?"}
C --> G
G -- true --> H["failDeploy with uploadResult.data.error"]
G -- false --> F
end
%% ---------- Edges ----------
A --> B
B --> C
B --> I
I --> J
The key modification is the new conditional branch on the response body to check for errors even if HTTP success is true. The schema evolves from a simple 3. Mermaid Diagram for Tech Debt & Proposed Improvementsflowchart TB
%% -------- Nodes --------
A["_deployCommand (massive function)"]
B["2000\+ lines of deploy.ts file (monolith)"]
C["Tech Debt: Difficult to maintain and test"]
D["Duplication: Spinner and error handling repeated"]
E["Loose typing between HTTP layers and CLI layers"]
F["Refactor: Break _deployCommand into smaller, testable functions"]
G["Improve types & validation on API Client responses"]
H["Use discriminated unions and exhaustive checks"]
I["Introduce handler classes or state machines for deployment stages"]
J["Better lifecycle and error management"]
%% -------- Links --------
A --> B
B --> C
B --> D
B --> E
C --> F
D --> F
E --> G
G --> H
F --> I
I --> J
%% -------- Clickable nodes --------
click F "https://github.com/triggerdotdev/cli/issues/123" "Example Refactor Issue"
click G "https://zod.dev" "Zod Validation Guide"
The massive Code Review & RoastingDeployment Changes
Schema Changes
SummaryThis PR thoughtfully addresses a neglected edge case in env var sync responses. The addition of proper discriminated union schemas and test coverage is a nice quality boost. However, the core You boys should really:
Otherwise, congrats! At least the code won't crash immediately in the face of weird env var server errors anymore. That's progress. If you want me to draw detailed call stacks, dependency graphs, or suggest automated code mod strategies to untangle your CLI deploy beast, just ask! |
This PR fixes the deployment error: "Failed to fetch environment variables: 200 Validation error: Required" that occurs when syncing environment variables during deployment.
Problem
The error occurred due to a schema mismatch between the server's error responses and the client's expectations:
/api/v1/projects/$projectRef/envvars/$slug/importreturns error responses like{ error: "...", variableErrors: [...] }with HTTP 200 statusEnvironmentVariableResponseBodyschema only accepted{ success: boolean }successfield was missingSolution
Schema Changes
EnvironmentVariableResponseBodyto be a union type that accepts both:{ success: true }{ error: string, variableErrors?: any[] }Client Logic Changes
Testing
The fix is minimal and surgical - it only affects the specific validation error without changing any other behavior, ensuring backward compatibility while resolving the deployment issue.
Fixes #1.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
binaries.prisma.shIf you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.