Skip to content

Commit 5fd978a

Browse files
committed
Fix race condition with sending a request while container is stopping
Due to some quirks in the runtime, it's possible for the DO to send a request to a container when it thinks the container is in a running state, but while the request is in flight, the container stops and the monitor promise resolves. This results in an error, and instead of retrying we throw a 500 error. Instead, recognize this case and restart the container. This is a bandage solution, but we will follow up with some improvements to the runtime that will clean up the state management required in this DO class.
1 parent 9885a4b commit 5fd978a

1 file changed

Lines changed: 113 additions & 3 deletions

File tree

src/lib/container.ts

Lines changed: 113 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const NO_CONTAINER_INSTANCE_ERROR =
2424
const RUNTIME_SIGNALLED_ERROR = 'runtime signalled the container to exit:';
2525
const UNEXPECTED_EXIT_ERROR = 'container exited with unexpected exit code:';
2626
const NOT_LISTENING_ERROR = 'the container is not listening';
27+
const STOPPING_CONTAINER_FAILED_ERROR = 'stopping container failed';
2728
const CONTAINER_STATE_KEY = '__CF_CONTAINER_STATE';
2829

2930
// maxRetries before scheduling next alarm is purposely set to 3,
@@ -77,6 +78,23 @@ const isRuntimeSignalledError = (error: unknown): boolean =>
7778
const isNotListeningError = (error: unknown): boolean => isErrorOfType(error, NOT_LISTENING_ERROR);
7879
const isContainerExitNonZeroError = (error: unknown): boolean =>
7980
isErrorOfType(error, UNEXPECTED_EXIT_ERROR);
81+
const isContainerNotRunningError = (error: unknown): boolean => {
82+
const patterns = [
83+
'the container is not running',
84+
'not expected to be running',
85+
'consider calling start()',
86+
];
87+
return patterns.some(pattern => isErrorOfType(error, pattern));
88+
};
89+
const isContainerCrashedDuringPortCheckError = (error: unknown): boolean =>
90+
isErrorOfType(error, 'container crashed while checking for ports');
91+
const isStoppingContainerFailedError = (error: unknown): boolean =>
92+
isErrorOfType(error, STOPPING_CONTAINER_FAILED_ERROR);
93+
const isRecoverableContainerUnavailableError = (error: unknown): boolean =>
94+
isContainerNotRunningError(error) ||
95+
isNotListeningError(error) ||
96+
isContainerCrashedDuringPortCheckError(error) ||
97+
isStoppingContainerFailedError(error);
8098

8199
function getExitCodeFromError(error: unknown): number | null {
82100
if (!(error instanceof Error)) {
@@ -512,7 +530,13 @@ export class Container<Env = Cloudflare.Env> extends DurableObject<Env> {
512530
*/
513531
public async stop(signal: Signal | SignalInteger = 'SIGTERM'): Promise<void> {
514532
if (this.container.running) {
515-
this.container.signal(typeof signal === 'string' ? signalToNumbers[signal] : signal);
533+
try {
534+
this.container.signal(typeof signal === 'string' ? signalToNumbers[signal] : signal);
535+
} catch (error) {
536+
if (!isRecoverableContainerUnavailableError(error)) {
537+
throw error;
538+
}
539+
}
516540
}
517541
await this.syncPendingStoppedEvents();
518542
}
@@ -690,7 +714,10 @@ export class Container<Env = Cloudflare.Env> extends DurableObject<Env> {
690714
const state = await this.state.getState();
691715
if (!this.container.running || state.status !== 'healthy') {
692716
try {
693-
await this.startAndWaitForPorts(port, { abort: request.signal });
717+
await this.startAndWaitForPortsWithRecovery(port, {
718+
abort: request.signal,
719+
context: 'startup',
720+
});
694721
} catch (e) {
695722
if (isNoInstanceError(e)) {
696723
return new Response(
@@ -721,6 +748,29 @@ export class Container<Env = Cloudflare.Env> extends DurableObject<Env> {
721748
throw e;
722749
}
723750

751+
// If container stopped or is no longer reachable during the request, restart and retry
752+
if (!this.container.running || isRecoverableContainerUnavailableError(e)) {
753+
try {
754+
await this.startAndWaitForPortsWithRecovery(port, {
755+
context: 'proxy',
756+
initialError: e,
757+
});
758+
const retryTcpPort = this.container.getTcpPort(port);
759+
return await retryTcpPort.fetch(containerUrl, request);
760+
} catch (retryError) {
761+
if (isNoInstanceError(retryError)) {
762+
return new Response(
763+
'There is no Container instance available at this time.\nThis is likely because you have reached your max concurrent instance count (set in wrangler config) or are you currently provisioning the Container.\nIf you are deploying your Container for the first time, check your dashboard to see provisioning status, this may take a few minutes.',
764+
{ status: 503 }
765+
);
766+
}
767+
return new Response(
768+
`Failed to restart container: ${retryError instanceof Error ? retryError.message : String(retryError)}`,
769+
{ status: 500 }
770+
);
771+
}
772+
}
773+
724774
// This error means that the container might've just restarted
725775
if (e.message.includes('Network connection lost.')) {
726776
return new Response('Container suddenly disconnected, try again', { status: 500 });
@@ -839,6 +889,56 @@ export class Container<Env = Cloudflare.Env> extends DurableObject<Env> {
839889
return { request, port };
840890
}
841891

892+
private async startAndWaitForPortsWithRecovery(
893+
port: number,
894+
options: {
895+
context: 'startup' | 'proxy';
896+
abort?: AbortSignal;
897+
initialError?: Error;
898+
}
899+
): Promise<void> {
900+
if (options.initialError) {
901+
console.debug(
902+
`Recoverable ${options.context} error for container ${this.ctx.id}, restarting and retrying: ${options.initialError.message}`
903+
);
904+
}
905+
906+
const attempts: (CancellationOptions | undefined)[] = [
907+
options.abort ? { abort: options.abort } : undefined,
908+
undefined,
909+
undefined,
910+
];
911+
912+
let lastError: unknown;
913+
914+
for (let index = 0; index < attempts.length; index++) {
915+
const cancellationOptions = attempts[index];
916+
917+
try {
918+
if (cancellationOptions) {
919+
await this.startAndWaitForPorts(port, cancellationOptions);
920+
} else {
921+
await this.startAndWaitForPorts(port);
922+
}
923+
return;
924+
} catch (error) {
925+
lastError = error;
926+
927+
if (!isRecoverableContainerUnavailableError(error) || index === attempts.length - 1) {
928+
throw error;
929+
}
930+
931+
console.debug(
932+
`Recoverable ${options.context} start error for container ${this.ctx.id}, retry ${index + 1}/${attempts.length - 1}: ${error instanceof Error ? error.message : String(error)}`
933+
);
934+
935+
await new Promise(resolve => setTimeout(resolve, 100));
936+
}
937+
}
938+
939+
throw lastError;
940+
}
941+
842942
/**
843943
*
844944
* The method prioritizes port sources in this order:
@@ -1138,7 +1238,17 @@ export class Container<Env = Cloudflare.Env> extends DurableObject<Env> {
11381238
}
11391239

11401240
if (this.isActivityExpired()) {
1141-
await this.onActivityExpired();
1241+
try {
1242+
await this.onActivityExpired();
1243+
} catch (error) {
1244+
if (!isRecoverableContainerUnavailableError(error)) {
1245+
throw error;
1246+
}
1247+
1248+
console.debug(
1249+
`Recoverable activity-expiration error for container ${this.ctx.id}: ${error instanceof Error ? error.message : String(error)}`
1250+
);
1251+
}
11421252
// renewActivityTimeout makes sure we don't spam calls here
11431253
this.renewActivityTimeout();
11441254
return;

0 commit comments

Comments
 (0)