Skip to content

Conversation

ntindle
Copy link
Member

@ntindle ntindle commented Sep 5, 2025

Sentry was not being enabled in dev/prod deployments because environment variables were being incorrectly overwritten during the Docker build process.

Changes 🏗️

  • Fixed Dockerfile environment variable merging logic to prevent .env.default from overwriting .env.production values
  • Added NODE_ENV=production to build stage to ensure Next.js looks for production env files
  • Updated env file merging to only run when not in CI/CD (when .env.production doesn't exist)
  • When .env.production exists (CI/CD), now merges defaults with production values properly

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • Verify local Docker builds still work with docker compose up
    • Verify dev deployment has NEXT_PUBLIC_APP_ENV=dev in built JavaScript
    • Verify prod deployment has NEXT_PUBLIC_APP_ENV=prod in built JavaScript
    • Verify Sentry is enabled in dev/prod deployments (isProdOrDev=true)

For configuration changes:

  • .env.default is updated or already compatible with my changes
  • docker-compose.yml is updated or already compatible with my changes
  • I have included a list of my configuration changes in the PR description (under Changes)

Technical Details

Root Cause:

  1. CI/CD workflow creates .env.production with correct values (e.g., NEXT_PUBLIC_APP_ENV=dev)
  2. Dockerfile's env merging logic always created .env from .env.default
  3. Next.js loads .env.production first, then .env second
  4. Since .env is loaded after .env.production, it overwrites the values
  5. .env.default has NEXT_PUBLIC_APP_ENV=local, causing getAppEnv() to return "local" instead of "dev"/"prod"
  6. This made isProdOrDev evaluate to false, disabling Sentry

Solution:
The Dockerfile now checks if .env.production exists:

  • If yes (CI/CD): Merges .env.default + .env.production.env.production (production values take precedence)
  • If no (local): Merges .env.default + .env.env (user values take precedence)

This ensures production deployments get the correct environment variables while preserving local development workflow.

🤖 Description generated + Investigation assisted with Claude Code

Co-Authored-By: Claude [email protected]

@ntindle ntindle requested a review from a team as a code owner September 5, 2025 22:38
@ntindle ntindle requested review from Pwuts and Bentlybro and removed request for a team September 5, 2025 22:38
@github-project-automation github-project-automation bot moved this to 🆕 Needs initial review in AutoGPT development kanban Sep 5, 2025
Copy link

netlify bot commented Sep 5, 2025

Deploy Preview for auto-gpt-docs-dev canceled.

Name Link
🔨 Latest commit f3483c6
🔍 Latest deploy log https://app.netlify.com/projects/auto-gpt-docs-dev/deploys/68bc676f954a060008f5b3c5

@github-actions github-actions bot added platform/frontend AutoGPT Platform - Front end conflicts Automatically applied to PRs with merge conflicts labels Sep 5, 2025
Copy link

netlify bot commented Sep 5, 2025

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit f3483c6
🔍 Latest deploy log https://app.netlify.com/projects/auto-gpt-docs/deploys/68bc676f661dfc0008dd0131

Copy link
Contributor

github-actions bot commented Sep 5, 2025

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot added the size/l label Sep 5, 2025
Copy link

deepsource-io bot commented Sep 5, 2025

Here's the code health analysis summary for commits c03af5c..f3483c6. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ Success
❗ 3 occurences introduced
🎯 3 occurences resolved
View Check ↗
DeepSource Python LogoPython✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

qodo-merge-pro bot commented Sep 5, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Noisy Console Logs

Multiple unconditional console.log statements were added for Sentry enablement diagnostics; consider gating them behind a debug flag or removing to reduce noise in production.

console.log(`shouldEnableSentry: ${shouldEnable} (instrumentation)`);
console.log(`isCloud: ${isCloud} (instrumentation)`);
console.log(`isDisabled: ${isDisabled} (instrumentation)`);
console.log(`AppEnv: ${getAppEnv()} (instrumentation)`);
console.log(`isProdOrDev: ${isProdOrDev} (instrumentation)`);
Env Merge Precedence

The merging step concatenates .env.default then .env.production; verify that duplicate keys from .env.default do not override downstream due to tooling quirks and that Next.js loads .env.production after merge as intended.

RUN if [ -f .env.production ]; then \
      # In CI/CD: merge defaults with production (production takes precedence)
      cat .env.default .env.production > .env.merged && mv .env.merged .env.production; \
    elif [ -f .env ]; then \
      # Local with custom .env: merge defaults with .env
      cat .env.default .env > .env.merged && mv .env.merged .env; \
    else \
      # Local without custom .env: use defaults
      cp .env.default .env; \
    fi
Env Source Ambiguity

getAppEnv now falls back to APP_ENV; confirm APP_ENV is not set in local shells/CI unintentionally, which could override NEXT_PUBLIC_APP_ENV and lead to unexpected behavior.

export function getAppEnv(): AppEnv {
  const env = process.env.NEXT_PUBLIC_APP_ENV || process.env.APP_ENV;
  if (env === "dev") return AppEnv.DEV;
  if (env === "prod") return AppEnv.PROD;
  // Some places use prod and others production
  if (env === "production") return AppEnv.PROD;

Copy link
Contributor

github-actions bot commented Sep 5, 2025

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot added size/m and removed conflicts Automatically applied to PRs with merge conflicts labels Sep 5, 2025
0ubbe
0ubbe previously approved these changes Sep 6, 2025
@github-project-automation github-project-automation bot moved this from 🆕 Needs initial review to 👍🏼 Mergeable in AutoGPT development kanban Sep 6, 2025
@Pwuts Pwuts enabled auto-merge September 6, 2025 16:58
@Pwuts Pwuts added this pull request to the merge queue Sep 6, 2025
Merged via the queue into dev with commit 0325ec0 Sep 6, 2025
33 checks passed
@Pwuts Pwuts deleted the ntindle/re-enable-sentry branch September 6, 2025 17:17
@github-project-automation github-project-automation bot moved this to Done in Frontend Sep 6, 2025
@github-project-automation github-project-automation bot moved this from 👍🏼 Mergeable to ✅ Done in AutoGPT development kanban Sep 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants