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

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
This commit is contained in:
Bjorn Lu 2023-08-03 22:26:24 +08:00 committed by GitHub
parent 68066290df
commit 1f0ee494a5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 27 additions and 30 deletions

View file

@ -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.

View file

@ -120,7 +120,3 @@ export async function startContainer({
return devServerAddressInfo; return devServerAddressInfo;
} }
export function isStarted(container: Container): boolean {
return !!container.viteServer.httpServer?.listening;
}

View file

@ -1,3 +1,3 @@
export { createContainer, isStarted, startContainer } from './container.js'; export { createContainer, startContainer } from './container.js';
export { default } from './dev.js'; export { default } from './dev.js';
export { createContainerWithAutomaticRestart } from './restart.js'; export { createContainerWithAutomaticRestart } from './restart.js';

View file

@ -10,12 +10,11 @@ import { createSafeError } from '../errors/index.js';
import { info, error as logError } from '../logger/core.js'; import { info, error as logError } from '../logger/core.js';
import { formatErrorMessage } from '../messages.js'; import { formatErrorMessage } from '../messages.js';
import type { Container } from './container'; import type { Container } from './container';
import { createContainer, isStarted, startContainer } from './container.js'; import { createContainer, startContainer } from './container.js';
async function createRestartedContainer( async function createRestartedContainer(
container: Container, container: Container,
settings: AstroSettings, settings: AstroSettings
needsStart: boolean
): Promise<Container> { ): Promise<Container> {
const { logging, fs, inlineConfig } = container; const { logging, fs, inlineConfig } = container;
const newContainer = await createContainer({ const newContainer = await createContainer({
@ -26,9 +25,7 @@ async function createRestartedContainer(
fs, fs,
}); });
if (needsStart) { await startContainer(newContainer);
await startContainer(newContainer);
}
return newContainer; return newContainer;
} }
@ -62,21 +59,15 @@ export function shouldRestartContainer(
return shouldRestart; return shouldRestart;
} }
export async function restartContainer( export async function restartContainer(container: Container): Promise<Container | Error> {
container: Container
): Promise<{ container: Container; error: Error | null }> {
const { logging, close, settings: existingSettings } = container; const { logging, close, settings: existingSettings } = container;
container.restartInFlight = true; container.restartInFlight = true;
const needsStart = isStarted(container);
try { try {
const { astroConfig } = await resolveConfig(container.inlineConfig, 'dev', container.fs); const { astroConfig } = await resolveConfig(container.inlineConfig, 'dev', container.fs);
const settings = createSettings(astroConfig, fileURLToPath(existingSettings.config.root)); const settings = createSettings(astroConfig, fileURLToPath(existingSettings.config.root));
await close(); await close();
return { return await createRestartedContainer(container, settings);
container: await createRestartedContainer(container, settings, needsStart),
error: null,
};
} catch (_err) { } catch (_err) {
const error = createSafeError(_err); const error = createSafeError(_err);
// Print all error messages except ZodErrors from AstroConfig as the pre-logged error is sufficient // 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 || '', stack: error.stack || '',
}, },
}); });
await close(); container.restartInFlight = false;
info(logging, 'astro', 'Continuing with previous valid configuration\n'); info(logging, 'astro', 'Continuing with previous valid configuration\n');
return { return error;
container: await createRestartedContainer(container, existingSettings, needsStart),
error,
};
} }
} }
@ -137,11 +125,16 @@ export async function createContainerWithAutomaticRestart({
async function handleServerRestart(logMsg: string) { async function handleServerRestart(logMsg: string) {
info(logging, 'astro', logMsg + '\n'); info(logging, 'astro', logMsg + '\n');
const container = restart.container; const container = restart.container;
const { container: newContainer, error } = await restartContainer(container); const result = await restartContainer(container);
restart.container = newContainer; if (result instanceof Error) {
// Add new watches because this is a new container with a new Vite server // Failed to restart, use existing container
addWatches(); resolveRestart(result);
resolveRestart(error); } 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) => { restartComplete = new Promise<Error | null>((resolve) => {
resolveRestart = resolve; resolveRestart = resolve;
}); });

View file

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