feat(api): environment variable parsing and validation #2255
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
5 issues found across 43 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="apps/api/.env.example">
<violation number="1" location="apps/api/.env.example:12">
`NUQ_DATABASE_URL`’s default points at port 5433, but the bundled Postgres service listens on 5432; with this default, the API cannot connect out of the box. Please align the example with the actual port.</violation>
</file>
<file name="apps/api/src/services/queue-worker.ts">
<violation number="1" location="apps/api/src/services/queue-worker.ts:309">
Binding the liveness server directly to config.WORKER_PORT ignores the platform-provided PORT fallback. Existing deployments that rely on only PORT (common on managed platforms) will now listen on the default 3005 and fail health checks. Please restore the PORT fallback when choosing the port.</violation>
</file>
<file name="apps/api/src/config.ts">
<violation number="1" location="apps/api/src/config.ts:34">
The boolean preprocessor turns every non-"true"/"1" value into false, so typos like ALLOW_LOCAL_WEBHOOKS=maybe pass validation silently instead of failing fast. Please return undefined for empty input, map known true/false strings, and leave other strings untouched so Zod can surface an explicit error.</violation>
<violation number="2" location="apps/api/src/config.ts:165">
Rule violated: **Enforce Use of Logger Instead of Console for Error Logging**
Replace the new `console.error` calls with the standard logger so production errors flow through the central logging pipeline [Rule: Enforce Use of Logger Instead of Console for Error Logging].</violation>
</file>
<file name="apps/api/src/lib/generic-ai.ts">
<violation number="1" location="apps/api/src/lib/generic-ai.ts:42">
Rule violated: **Enforce Use of Logger Instead of Console for Error Logging**
Replace this console.error call with the standard logger to comply with our logging policy for production error handling.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
apps/api/.env.example
Outdated
| REDIS_RATE_LIMIT_URL=redis://redis:6379 | ||
|
|
||
| # For self-hosting using docker, use postgres://postgres:postgres@nuq-postgres:5432/postgres. For running locally, use postgres://postgres:postgres@localhost:5433/postgres | ||
| NUQ_DATABASE_URL=postgres://postgres:postgres@nuq-postgres:5433/postgres |
There was a problem hiding this comment.
NUQ_DATABASE_URL’s default points at port 5433, but the bundled Postgres service listens on 5432; with this default, the API cannot connect out of the box. Please align the example with the actual port.
Prompt for AI agents
Address the following comment on apps/api/.env.example at line 12:
<comment>`NUQ_DATABASE_URL`’s default points at port 5433, but the bundled Postgres service listens on 5432; with this default, the API cannot connect out of the box. Please align the example with the actual port.</comment>
<file context>
@@ -1,100 +1,141 @@
+REDIS_RATE_LIMIT_URL=redis://redis:6379
+
+# For self-hosting using docker, use postgres://postgres:postgres@nuq-postgres:5432/postgres. For running locally, use postgres://postgres:postgres@localhost:5433/postgres
+NUQ_DATABASE_URL=postgres://postgres:postgres@nuq-postgres:5433/postgres
+
+# For using the Playwright microservice, set the URL here (set to empty if you are not using Playwright)
</file context>
Suggested change
| NUQ_DATABASE_URL=postgres://postgres:postgres@nuq-postgres:5433/postgres | |
| NUQ_DATABASE_URL=postgres://postgres:postgres@nuq-postgres:5432/postgres |
✅ Addressed in 60c1341
| const workerPort = process.env.WORKER_PORT || process.env.PORT || 3005; | ||
| app.listen(workerPort, () => { | ||
| _logger.info(`Liveness endpoint is running on port ${workerPort}`); | ||
| app.listen(config.WORKER_PORT, () => { |
There was a problem hiding this comment.
Binding the liveness server directly to config.WORKER_PORT ignores the platform-provided PORT fallback. Existing deployments that rely on only PORT (common on managed platforms) will now listen on the default 3005 and fail health checks. Please restore the PORT fallback when choosing the port.
Prompt for AI agents
Address the following comment on apps/api/src/services/queue-worker.ts at line 309:
<comment>Binding the liveness server directly to config.WORKER_PORT ignores the platform-provided PORT fallback. Existing deployments that rely on only PORT (common on managed platforms) will now listen on the default 3005 and fail health checks. Please restore the PORT fallback when choosing the port.</comment>
<file context>
@@ -312,9 +306,8 @@ app.get("/liveness", (req, res) => {
-const workerPort = process.env.WORKER_PORT || process.env.PORT || 3005;
-app.listen(workerPort, () => {
- _logger.info(`Liveness endpoint is running on port ${workerPort}`);
+app.listen(config.WORKER_PORT, () => {
+ _logger.info(`Liveness endpoint is running on port ${config.WORKER_PORT}`);
});
</file context>
apps/api/src/config.ts
Outdated
| const strbool = (defaultValue: boolean) => | ||
| z | ||
| .preprocess( | ||
| val => (val === "true" || val === "1" ? true : false), |
There was a problem hiding this comment.
The boolean preprocessor turns every non-"true"/"1" value into false, so typos like ALLOW_LOCAL_WEBHOOKS=maybe pass validation silently instead of failing fast. Please return undefined for empty input, map known true/false strings, and leave other strings untouched so Zod can surface an explicit error.
Prompt for AI agents
Address the following comment on apps/api/src/config.ts at line 34:
<comment>The boolean preprocessor turns every non-"true"/"1" value into false, so typos like ALLOW_LOCAL_WEBHOOKS=maybe pass validation silently instead of failing fast. Please return undefined for empty input, map known true/false strings, and leave other strings untouched so Zod can surface an explicit error.</comment>
<file context>
@@ -0,0 +1,180 @@
+const strbool = (defaultValue: boolean) =>
+ z
+ .preprocess(
+ val => (val === "true" || val === "1" ? true : false),
+ z.boolean(),
+ )
</file context>
apps/api/src/config.ts
Outdated
| // } | ||
| } catch (err) { | ||
| if (err instanceof z.ZodError) { | ||
| console.error("Problems found with environment variables:"); |
There was a problem hiding this comment.
Rule violated: Enforce Use of Logger Instead of Console for Error Logging
Replace the new console.error calls with the standard logger so production errors flow through the central logging pipeline [Rule: Enforce Use of Logger Instead of Console for Error Logging].
Prompt for AI agents
Address the following comment on apps/api/src/config.ts at line 165:
<comment>Replace the new `console.error` calls with the standard logger so production errors flow through the central logging pipeline [Rule: Enforce Use of Logger Instead of Console for Error Logging].</comment>
<file context>
@@ -0,0 +1,180 @@
+ // }
+} catch (err) {
+ if (err instanceof z.ZodError) {
+ console.error("Problems found with environment variables:");
+ for (const issue of err.issues) {
+ if (issue.path && issue.path.length > 0) {
</file context>
✅ Addressed in 60c1341
apps/api/src/lib/generic-ai.ts
Outdated
| keyFile: vertexCredentials, | ||
| }; | ||
| } else { | ||
| console.error( |
There was a problem hiding this comment.
Rule violated: Enforce Use of Logger Instead of Console for Error Logging
Replace this console.error call with the standard logger to comply with our logging policy for production error handling.
Prompt for AI agents
Address the following comment on apps/api/src/lib/generic-ai.ts at line 42:
<comment>Replace this console.error call with the standard logger to comply with our logging policy for production error handling.</comment>
<file context>
@@ -18,46 +20,71 @@ type Provider =
+ keyFile: vertexCredentials,
+ };
+ } else {
+ console.error(
+ "Failed to parse VERTEX_CREDENTIALS environment variable. It should be a base64-encoded JSON string or a valid file path. Failing back to default keyFile.",
+ );
</file context>
✅ Addressed in 8c7d9eb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improved environment variable configuration system
Summary by cubic
Centralized environment configuration with validation at startup to prevent misconfigurations, plus refactors to use a typed config across the API. This improves reliability, clearer errors, and safer defaults for self-hosted setups.
New Features
Migration