Fix failed server restart calling integration hooks twice (#7917)

Co-authored-by: Emanuele Stoppa <[email protected]>
withastro · Aug 3, 2023 · 1f0ee49 · 1f0ee49
1 parent 6806629
commit 1f0ee49
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 30 deletions.
diff --git a/.changeset/metal-chairs-beam.md b/.changeset/metal-chairs-beam.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Prevent integration hooks from re-triggering if the server restarts on config change, but the config fails to load.
diff --git a/packages/astro/src/core/dev/container.ts b/packages/astro/src/core/dev/container.ts
@@ -120,7 +120,3 @@ export async function startContainer({
 
 	return devServerAddressInfo;
 }
-
-export function isStarted(container: Container): boolean {
-	return !!container.viteServer.httpServer?.listening;
-}
diff --git a/packages/astro/src/core/dev/index.ts b/packages/astro/src/core/dev/index.ts
@@ -1,3 +1,3 @@
-export { createContainer, isStarted, startContainer } from './container.js';
+export { createContainer, startContainer } from './container.js';
 export { default } from './dev.js';
 export { createContainerWithAutomaticRestart } from './restart.js';
diff --git a/packages/astro/src/core/dev/restart.ts b/packages/astro/src/core/dev/restart.ts
@@ -10,12 +10,11 @@ import { createSafeError } from '../errors/index.js';
 import { info, error as logError } from '../logger/core.js';
 import { formatErrorMessage } from '../messages.js';
 import type { Container } from './container';
-import { createContainer, isStarted, startContainer } from './container.js';
+import { createContainer, startContainer } from './container.js';
 
 async function createRestartedContainer(
 	container: Container,
-	settings: AstroSettings,
-	needsStart: boolean
+	settings: AstroSettings
 ): Promise<Container> {
 	const { logging, fs, inlineConfig } = container;
 	const newContainer = await createContainer({
@@ -26,9 +25,7 @@ async function createRestartedContainer(
 		fs,
 	});
 
-	if (needsStart) {
-		await startContainer(newContainer);
-	}
+	await startContainer(newContainer);
 
 	return newContainer;
 }
@@ -62,21 +59,15 @@ export function shouldRestartContainer(
 	return shouldRestart;
 }
 
-export async function restartContainer(
-	container: Container
-): Promise<{ container: Container; error: Error | null }> {
+export async function restartContainer(container: Container): Promise<Container | Error> {
 	const { logging, close, settings: existingSettings } = container;
 	container.restartInFlight = true;
 
-	const needsStart = isStarted(container);
 	try {
 		const { astroConfig } = await resolveConfig(container.inlineConfig, 'dev', container.fs);
 		const settings = createSettings(astroConfig, fileURLToPath(existingSettings.config.root));
 		await close();
-		return {
-			container: await createRestartedContainer(container, settings, needsStart),
-			error: null,
-		};
+		return await createRestartedContainer(container, settings);
 	} catch (_err) {
 		const error = createSafeError(_err);
 		// Print all error messages except ZodErrors from AstroConfig as the pre-logged error is sufficient
@@ -91,12 +82,9 @@ export async function restartContainer(
 				stack: error.stack || '',
 			},
 		});
-		await close();
+		container.restartInFlight = false;
 		info(logging, 'astro', 'Continuing with previous valid configuration\n');
-		return {
-			container: await createRestartedContainer(container, existingSettings, needsStart),
-			error,
-		};
+		return error;
 	}
 }
 
@@ -137,11 +125,16 @@ export async function createContainerWithAutomaticRestart({
 	async function handleServerRestart(logMsg: string) {
 		info(logging, 'astro', logMsg + '\n');
 		const container = restart.container;
-		const { container: newContainer, error } = await restartContainer(container);
-		restart.container = newContainer;
-		// Add new watches because this is a new container with a new Vite server
-		addWatches();
-		resolveRestart(error);
+		const result = await restartContainer(container);
+		if (result instanceof Error) {
+			// Failed to restart, use existing container
+			resolveRestart(result);
+		} else {
+			// Restart success. Add new watches because this is a new container with a new Vite server
+			restart.container = result;
+			addWatches();
+			resolveRestart(null);
+		}
 		restartComplete = new Promise<Error | null>((resolve) => {
 			resolveRestart = resolve;
 		});

diff --git a/packages/astro/test/units/dev/restart.test.js b/packages/astro/test/units/dev/restart.test.js
@@ -4,13 +4,16 @@ import { fileURLToPath } from 'node:url';
 
 import {
 	createContainerWithAutomaticRestart,
-	isStarted,
 	startContainer,
 } from '../../../dist/core/dev/index.js';
 import { createFs, createRequestAndResponse, triggerFSEvent } from '../test-utils.js';
 
 const root = new URL('../../fixtures/alias/', import.meta.url);
 
+function isStarted(container) {
+	return !!container.viteServer.httpServer?.listening;
+}
+
 describe('dev container restarts', () => {
 	it('Surfaces config errors on restarts', async () => {
 		const fs = createFs(