Fail recipe when npm install fails (#7227)

Author: greg-at-moderne

rewrite-javascript/rewrite/src/javascript/recipes/upgrade-dependency-version.ts

@@ -31,7 +31,7 @@ import {
 import * as path from "path";
 import * as semver from "semver";
 import * as picomatch from "picomatch";
-import {markupWarn, replaceMarkerByKind} from "../../markers";
+import {replaceMarkerByKind} from "../../markers";
 import {TreePrinters} from "../../print";
 import {
     createDependencyRecipeAccumulator,
@@ -301,11 +301,7 @@ export class UpgradeDependencyVersion extends ScanningRecipe<Accumulator> {
                         );
                     if (failureMessage) {
                         const names = updateInfo.matchedDependencies.map(d => d.packageName).join(', ');
-                        return markupWarn(
-                            doc,
-                            `Failed to upgrade ${names} to ${recipe.newVersion}`,
-                            failureMessage
-                        );
+                        throw new Error(`Failed to upgrade ${names} to ${recipe.newVersion}: ${failureMessage}`);
                     }
 
                     let modifiedDoc = doc;

rewrite-javascript/rewrite/test/javascript/recipes/upgrade-dependency-version.test.ts

@@ -24,7 +24,8 @@ import {
 import {Json} from "../../../src/json";
 import {RecipeSpec} from "../../../src/test";
 import {withDir} from "tmp-promise";
-import {findMarker, MarkersKind} from "../../../src";
+import * as fs from "fs";
+import * as path from "path";
 
 describe("UpgradeDependencyVersion", () => {
 
@@ -292,40 +293,62 @@ describe("UpgradeDependencyVersion", () => {
         }, {unsafeCleanup: true});
     });
 
-    test("adds warning marker when version does not exist", async () => {
+    test("throws when install fails for non-existent version", async () => {
         const spec = new RecipeSpec();
         spec.recipe = new UpgradeDependencyVersion({
             packageName: "uuid",
             newVersion: "^999.0.0" // Non-existent version
         });
 
         await withDir(async (repo) => {
-            await spec.rewriteRun(
+            await expect(spec.rewriteRun(
                 npm(
                     repo.path,
                     typescript(`const x = 1;`),
-                    {
-                        // Version doesn't change, but warning marker is added
-                        ...packageJson(`
+                    packageJson(`
                         {
                             "name": "test-project",
                             "version": "1.0.0",
                             "dependencies": {
                                 "uuid": "^9.0.0"
                             }
                         }
-                    `, (actual: string) => {
-                            expect(actual).toContain('/*~~(Failed to upgrade uuid to ^999.0.0');
-                            return actual;
-                        }), afterRecipe: async (doc: Json.Document) => {
-                            // Should have a warning marker
-                            const warnMarker = findMarker(doc, MarkersKind.MarkupWarn);
-                            expect(warnMarker).toBeDefined();
-                            expect((warnMarker as any).message).toContain("Failed to upgrade uuid");
+                    `)
+                )
+            )).rejects.toThrow("Failed to upgrade uuid to ^999.0.0");
+        }, {unsafeCleanup: true});
+    });
+
+    test("throws when install fails due to engine version mismatch", async () => {
+        const spec = new RecipeSpec();
+        spec.recipe = new UpgradeDependencyVersion({
+            packageName: "uuid",
+            newVersion: "^10.0.0"
+        });
+
+        await withDir(async (repo) => {
+            // given
+            fs.writeFileSync(path.join(repo.path, '.npmrc'), 'engine-strict=true');
+
+            // when / then
+            await expect(spec.rewriteRun(
+                npm(
+                    repo.path,
+                    typescript(`const x = 1;`),
+                    packageJson(`
+                        {
+                            "name": "test-project",
+                            "version": "1.0.0",
+                            "engines": {
+                                "node": "504.436"
+                            },
+                            "dependencies": {
+                                "uuid": "^9.0.0"
+                            }
                         }
-                    }
+                    `)
                 )
-            );
+            )).rejects.toThrow(/^Error: Failed to upgrade uuid to \^10\.0\.0:/);
         }, {unsafeCleanup: true});
     });