From 5b92dfd85ff056da7663d2e6f78b5748fc7529e2 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@meteor.com>
Date: Mon, 25 Aug 2014 11:16:42 -0700
Subject: [PATCH] "better" error handling for compiler constraints

This before was just an uncaught exception. Now it's exit(1), which is
bad too.  This should just use buildmessage, but for some reason that
doesn't work here.
---
 tools/compiler.js | 46 ++++++++++++++++++++++++++++++++++++----------
 tools/project.js  |  2 ++
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/tools/compiler.js b/tools/compiler.js
index dd36c50120..3f1145fb1c 100644
--- a/tools/compiler.js
+++ b/tools/compiler.js
@@ -187,11 +187,24 @@ var determineBuildTimeDependencies = function (packageSource,
   });
 
   var versions = packageSource.dependencyVersions.dependencies || {};
-  ret.packageDependencies =
-    packageSource.catalog.resolveConstraints(
-      constraints_array,
-      { previousSolution: versions },
-      constraintSolverOpts);
+  try {
+    ret.packageDependencies =
+      packageSource.catalog.resolveConstraints(
+        constraints_array,
+        { previousSolution: versions },
+        constraintSolverOpts);
+  } catch (e) {
+    if (!e.constraintSolverError)
+      throw e;
+    // XXX should use buildmessage here, but the code isn't structured properly
+    // to ignore issues.  eg, try "meteor publish" where the onTest depends on a
+    // nonexistent package.  see the other call in this function too, and
+    // project._ensureDepsUpToDate
+    process.stderr.write(
+      "Could not resolve the specified constraints for this package:\n"
+        + e + "\n");
+    process.exit(1);
+  }
 
   // We care about differentiating between all dependencies (which we save in
   // the version lock file) and the direct dependencies (which are packages that
@@ -226,11 +239,24 @@ var determineBuildTimeDependencies = function (packageSource,
     });
 
     var pluginVersion = pluginVersions[info.name] || {};
-    ret.pluginDependencies[info.name] =
-      packageSource.catalog.resolveConstraints(
-        constraints_array,
-        { previousSolution: pluginVersion },
-        constraintSolverOpts );
+    try {
+      ret.pluginDependencies[info.name] =
+        packageSource.catalog.resolveConstraints(
+          constraints_array,
+          { previousSolution: pluginVersion },
+          constraintSolverOpts );
+    } catch (e) {
+      if (!e.constraintSolverError)
+        throw e;
+      // XXX should use buildmessage here, but the code isn't structured
+      // properly to ignore issues.  eg, try "meteor publish" where the onTest
+      // depends on a nonexistent package.  see the other call in this function
+      // too, and project._ensureDepsUpToDate
+      process.stderr.write(
+        "Could not resolve the specified constraints for this package:\n"
+          + e + "\n");
+      process.exit(1);
+    }
   });
 
   // Every time we run the constraint solver, we record the results. This has
diff --git a/tools/project.js b/tools/project.js
index c94bd438bd..80bf81f29c 100644
--- a/tools/project.js
+++ b/tools/project.js
@@ -185,6 +185,8 @@ _.extend(Project.prototype, {
           { ignoreProjectDeps: true }
         );
       } catch (err) {
+        // XXX This error handling is bogus. Use buildmessage instead, or
+        // something. See also compiler.determineBuildTimeDependencies
         process.stdout.write(
           "Could not resolve the specified constraints for this project:\n"
            + (err.constraintSolverError ? err : err.stack) + "\n");