Skip to content

Commit 87b8f30

Browse files
gianniguidaclaude
andauthored
[2.x] fix: Handle network connection loss in the frontend (#4854)
* feat: handle network connection loss in the frontend Port of the 1.x network-loss handling (#4843) to 2.x: - Classify network-level request failures (status 0) in requestErrorCatch and show a dedicated translated alert (offline-specific when navigator.onLine is false; failed cross-origin requests keep their existing message). Parallel failures produce a single alert. Aborted requests cannot trigger this, as Mithril leaves their promises unsettled. - Listen to the window online/offline events: going offline shows a persistent dismissible alert; reconnecting clears connection alerts and briefly confirms. - Defer GET requests that fail while the browser is offline: their promises are held open and settled with the result of a retry once connectivity is restored, so content that failed to load appears automatically without a page reload. Writes keep failing fast. Identical deferred requests (e.g. from a polling widget) are keyed by method, URL and params, and retried with a single request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: retry lazy chunk loads that failed while offline Code-split chunks load via script tags, not XHR, so the offline handling in Application#request never sees them. A chunk that failed to load while offline previously rejected (or was silently swallowed by its caller): the reply/discussion composer would not open, and DiscussionPage's PostStream import left the page stuck on its loading skeleton with no retry — even after connectivity returned. Since every chunk load funnels through ExportRegistry#loadChunk (webpack's script loader is overridden with it), hold back failures that occur while the browser reports being offline and retry once the online event fires. The import() promise stays pending in the meantime, so every caller — composer bodies, lazy routes, modals, the post stream — recovers on its own, without changes to call sites. Failures while the browser is online are reported unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style: format Application.tsx with the repo's pinned prettier * test: silence expected chunk-url warning in ExportRegistry tests The tests never register chunks, so the registry legitimately warns before falling back to the URL passed in. Spy it away, as the admin Application test does for console.group/error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: assert the unregistered-chunk warning and URL fallback Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent ba11aa9 commit 87b8f30

5 files changed

Lines changed: 680 additions & 13 deletions

File tree

framework/core/js/src/common/Application.tsx

Lines changed: 175 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,33 @@ export default class Application {
304304
*/
305305
private requestErrorAlert: number | null = null;
306306

307+
/**
308+
* The key for the Alert that was shown as a result of an AJAX request
309+
* failing at the network level (status 0). Unlike other request error
310+
* alerts, only one of these is shown at a time.
311+
*/
312+
protected networkErrorAlert: number | null = null;
313+
314+
/**
315+
* The key for the Alert that is shown while the browser reports being
316+
* offline.
317+
*/
318+
protected offlineAlert: number | null = null;
319+
320+
/**
321+
* GET requests that failed because the browser was offline, keyed by
322+
* method, URL and params. They are retried, and their original promises
323+
* settled, once connectivity is restored. Identical requests (e.g. from a
324+
* polling extension) share one entry and are retried only once.
325+
*/
326+
protected deferredRequests: Map<
327+
string,
328+
{
329+
options: FlarumRequestOptions<any>;
330+
settlers: { resolve: (value: any) => void; reject: (error: unknown) => void }[];
331+
}
332+
> = new Map();
333+
307334
initialRoute!: string;
308335

309336
/**
@@ -358,6 +385,8 @@ export default class Application {
358385

359386
this.mount();
360387

388+
this.registerConnectivityListeners();
389+
361390
this.initialRoute = window.location.href;
362391

363392
caughtInitializationErrors.forEach((handler) => handler());
@@ -667,7 +696,61 @@ export default class Application {
667696

668697
if (this.requestErrorAlert) this.alerts.dismiss(this.requestErrorAlert);
669698

670-
return m.request(options).catch((e) => this.requestErrorCatch(e, originalOptions.errorHandler));
699+
if (this.networkErrorAlert) {
700+
this.alerts.dismiss(this.networkErrorAlert);
701+
this.networkErrorAlert = null;
702+
}
703+
704+
return m.request(options).catch((e) => {
705+
if (this.shouldDeferRequest(e, originalOptions)) {
706+
return this.deferRequest(originalOptions);
707+
}
708+
709+
return this.requestErrorCatch(e, originalOptions.errorHandler);
710+
});
711+
}
712+
713+
/**
714+
* Whether a failed request should be held back and retried once
715+
* connectivity is restored, instead of being rejected.
716+
*
717+
* Only GET requests that failed at the network level while the browser
718+
* reported being offline qualify: they are safe to repeat, and the
719+
* `online` event provides a reliable signal to retry them.
720+
*/
721+
protected shouldDeferRequest(error: unknown, originalOptions: FlarumRequestOptions<any>): boolean {
722+
return (
723+
error instanceof RequestError && error.status === 0 && navigator.onLine === false && (originalOptions.method ?? 'GET').toUpperCase() === 'GET'
724+
);
725+
}
726+
727+
/**
728+
* Hold a request that failed while offline. The returned promise settles
729+
* with the result of retrying the request once connectivity is restored.
730+
*/
731+
protected deferRequest<ResponseType>(originalOptions: FlarumRequestOptions<ResponseType>): Promise<ResponseType> {
732+
// Make sure the offline alert is showing, in case the page was loaded
733+
// while already offline and no `offline` event was ever fired.
734+
this.connectionLost();
735+
736+
return new Promise((resolve, reject) => {
737+
const key = this.deferredRequestKey(originalOptions);
738+
const deferred = this.deferredRequests.get(key);
739+
740+
if (deferred) {
741+
deferred.settlers.push({ resolve, reject });
742+
} else {
743+
this.deferredRequests.set(key, { options: originalOptions, settlers: [{ resolve, reject }] });
744+
}
745+
});
746+
}
747+
748+
/**
749+
* The identity of a request for deferral purposes: requests with the same
750+
* key are considered identical and are retried only once.
751+
*/
752+
protected deferredRequestKey(options: FlarumRequestOptions<any>): string {
753+
return [options.method ?? 'GET', options.url, JSON.stringify(options.params ?? null)].join(' ');
671754
}
672755

673756
/**
@@ -682,6 +765,20 @@ export default class Application {
682765

683766
let content;
684767
switch (error.status) {
768+
// Status 0 means the request failed at the network level: the client is
769+
// offline, DNS resolution failed, the origin was unreachable, or the
770+
// response was blocked by CORS. Aborted requests never get here, as
771+
// Mithril leaves their promises unsettled.
772+
case 0:
773+
if (navigator.onLine === false) {
774+
content = app.translator.trans('core.lib.error.offline_message');
775+
} else if (this.requestWasCrossOrigin(error)) {
776+
content = app.translator.trans('core.lib.error.generic_cross_origin_message');
777+
} else {
778+
content = app.translator.trans('core.lib.error.network_message');
779+
}
780+
break;
781+
685782
case 422:
686783
content = formattedErrors
687784
.map((detail) => [detail, <br />])
@@ -780,13 +877,89 @@ export default class Application {
780877
if (e.status === 500 && isDebug) {
781878
app.modal.show(RequestErrorModal, { error: e, formattedError: formattedErrors });
782879
} else if (e.alert) {
783-
this.requestErrorAlert = this.alerts.show(e.alert, e.alert.content);
880+
if (e.status === 0) {
881+
// A connection problem produces a single alert, even if several
882+
// parallel requests fail at once.
883+
if (!this.connectionAlertActive()) {
884+
this.networkErrorAlert = this.alerts.show(e.alert, e.alert.content);
885+
}
886+
} else {
887+
this.requestErrorAlert = this.alerts.show(e.alert, e.alert.content);
888+
}
784889
}
785890
} else {
786891
throw e;
787892
}
788893
}
789894

895+
/**
896+
* Whether an alert about a connection problem (a network-level request
897+
* failure or the browser being offline) is currently being shown.
898+
*/
899+
protected connectionAlertActive(): boolean {
900+
const activeAlerts = this.alerts.getActiveAlerts();
901+
902+
return (
903+
(this.networkErrorAlert !== null && this.networkErrorAlert in activeAlerts) || (this.offlineAlert !== null && this.offlineAlert in activeAlerts)
904+
);
905+
}
906+
907+
/**
908+
* Register listeners to proactively notify the user when the browser goes
909+
* offline and when connectivity is restored.
910+
*/
911+
protected registerConnectivityListeners(): void {
912+
window.addEventListener('offline', () => this.connectionLost());
913+
window.addEventListener('online', () => this.connectionRestored());
914+
}
915+
916+
/**
917+
* Show a persistent (but dismissible) alert while the browser reports being
918+
* offline.
919+
*/
920+
protected connectionLost(): void {
921+
if (this.offlineAlert !== null && this.offlineAlert in this.alerts.getActiveAlerts()) return;
922+
923+
// The offline alert supersedes any alert shown for a failed request.
924+
if (this.networkErrorAlert !== null) {
925+
this.alerts.dismiss(this.networkErrorAlert);
926+
this.networkErrorAlert = null;
927+
}
928+
929+
this.offlineAlert = this.alerts.show({ type: 'error', dismissible: true }, app.translator.trans('core.lib.error.offline_message'));
930+
}
931+
932+
/**
933+
* Dismiss any connection problem alerts, retry requests that were deferred
934+
* while offline, and briefly confirm to the user that connectivity has
935+
* been restored.
936+
*/
937+
protected connectionRestored(): void {
938+
if (this.networkErrorAlert !== null) {
939+
this.alerts.dismiss(this.networkErrorAlert);
940+
this.networkErrorAlert = null;
941+
}
942+
943+
const deferred = Array.from(this.deferredRequests.values());
944+
this.deferredRequests = new Map();
945+
946+
deferred.forEach(({ options, settlers }) => {
947+
this.request(options).then(
948+
(response) => settlers.forEach(({ resolve }) => resolve(response)),
949+
(error) => settlers.forEach(({ reject }) => reject(error))
950+
);
951+
});
952+
953+
if (this.offlineAlert === null) return;
954+
955+
this.alerts.dismiss(this.offlineAlert);
956+
this.offlineAlert = null;
957+
958+
const confirmationAlert = this.alerts.show({ type: 'success' }, app.translator.trans('core.lib.connection_restored_message'));
959+
960+
setTimeout(() => this.alerts.dismiss(confirmationAlert), 10000);
961+
}
962+
790963
private showDebug(error: RequestError, formattedError: string[]) {
791964
if (this.requestErrorAlert !== null) this.alerts.dismiss(this.requestErrorAlert);
792965

framework/core/js/src/common/ExportRegistry.ts

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -172,17 +172,40 @@ export default class ExportRegistry implements IExportRegistry, IChunkRegistry {
172172
// @ts-ignore
173173
app.alerts.showLoading();
174174

175-
return await original(
176-
this.chunkUrl(chunkId) || url,
177-
(...args: any) => {
178-
// @ts-ignore
179-
app.alerts.clearLoading();
180-
181-
return done(...args);
182-
},
183-
key,
184-
chunkId
185-
);
175+
const chunkUrl = this.chunkUrl(chunkId) || url;
176+
177+
const load = (): Promise<void> =>
178+
original(
179+
chunkUrl,
180+
(...args: any) => {
181+
const event: Event | undefined = args[0];
182+
183+
// A chunk that failed to load because the browser is offline is
184+
// retried once connectivity is restored. Its import() promise stays
185+
// pending in the meantime, so every caller awaiting the chunk
186+
// recovers on its own — mirroring how `Application#request` defers
187+
// GET requests that fail while offline.
188+
if (event?.type === 'error' && navigator.onLine === false) {
189+
const retry = () => {
190+
window.removeEventListener('online', retry);
191+
load();
192+
};
193+
194+
window.addEventListener('online', retry);
195+
196+
return;
197+
}
198+
199+
// @ts-ignore
200+
app.alerts.clearLoading();
201+
202+
return done(...args);
203+
},
204+
key,
205+
chunkId
206+
);
207+
208+
return await load();
186209
}
187210

188211
chunkUrl(chunkId: number | string): string | null {

0 commit comments

Comments
 (0)