refactoring-backend #3

Merged
lukas.pupkalipinski merged 403 commits from refactoring-backend into main 2026-05-20 20:23:46 +02:00
15 changed files with 291 additions and 73 deletions
Showing only changes of commit 96a21ffb70 - Show all commits

View File

@@ -1,40 +1,3 @@
## [IMPORTANT] Provider ordering fragility (Frontend)
**Where found**
- `frontend/src/App.tsx` — 10-level deep provider nesting
- `frontend/src/providers/PROVIDER_ORDER.md` — documents order, no compile-time enforcement
**Why this is needed**
Provider order (ThemeProvider → AppContents → FluentProvider → ...) enforced only at runtime. Accidental reorder caught only after deploy.
**Goal**
Add compile-time validation of provider ordering.
**What to do**
1. Create provider composition utility enforcing order
2. Use TypeScript discriminated unions
3. Add ESLint rule to check provider wrapping
**Possible traps and issues**
- TypeScript doesn't easily enforce ordering
- May be overkill — improve runtime error messages instead
**Docs changes needed**
- Update `Docs/Architekture.md` § 3.2 (Providers)
**Doc references**
- `Docs/Architekture.md` § 3.2 (Providers)
- `frontend/src/providers/PROVIDER_ORDER.md`
---
## [IMPORTANT] Promise cancellation not checked in .then()/.catch() chains
**Where found**

View File

@@ -1046,6 +1046,64 @@ function useBans(hours: number): UseBansResult {
export default useBans;
```
### Promise Cancellation in Callbacks
When performing async operations in `useCallback` (form submissions, button clicks, etc.), always check if the operation was cancelled before updating state. This prevents React warnings and memory leaks when components unmount or users navigate away.
**Problem with naked `.then()` chains:**
```tsx
const handleSubmit = useCallback((values: FormValues) => {
setSaving(true);
saveData(values)
.then(() => {
setSaving(false); // ❌ Updates state even if component unmounted
setDialogOpen(false);
})
.catch((err) => {
setSaving(false); // ❌ Updates state on unmounted component
setError(err.message);
});
}, []);
```
**Solution: Use AbortController to track and cancel operations:**
```tsx
const submitControllerRef = useRef<AbortController | null>(null);
const handleSubmit = useCallback((values: FormValues) => {
// Abort any previous in-flight request
submitControllerRef.current?.abort();
const controller = new AbortController();
submitControllerRef.current = controller;
setSaving(true);
saveData(values)
.then(() => {
if (controller.signal.aborted) return; // ✅ Check before state updates
setSaving(false);
setDialogOpen(false);
})
.catch((err) => {
if (controller.signal.aborted) return; // ✅ Check before state updates
setSaving(false);
setError(err.message);
});
}, []);
// Clean up on unmount
useEffect(() => {
return () => {
submitControllerRef.current?.abort(); // ✅ Abort in cleanup
};
}, []);
```
**Key rules:**
- Always create a **local `const`** variable to capture the controller — never read `ref.current` inside `.then()` callbacks (race condition risk).
- Check `controller.signal.aborted` **in every `.then()` and `.catch()` block** before calling `setState`.
- Abort any previous operation before starting a new one to avoid stale callbacks.
- Clean up with a `useEffect` return function on component unmount.
### AbortController in Hooks
When using `AbortController` for fetch cancellation in hooks with mutable refs:

View File

@@ -27,6 +27,7 @@ export function BlocklistScheduleSection({ onRunImport, runImportRunning, import
const [saving, setSaving] = useState(false);
const [saveMsg, setSaveMsg] = useState<string | null>(null);
const saveTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const submitControllerRef = useRef<AbortController | null>(null);
const config = info?.config ?? {
frequency: "daily" as ScheduleFrequency,
@@ -44,18 +45,25 @@ export function BlocklistScheduleSection({ onRunImport, runImportRunning, import
saveTimeoutRef.current = null;
}
submitControllerRef.current?.abort();
const controller = new AbortController();
submitControllerRef.current = controller;
setSaving(true);
saveSchedule(draft)
.then(() => {
if (controller.signal.aborted) return;
refresh();
setSaveMsg("Schedule saved.");
setSaving(false);
saveTimeoutRef.current = setTimeout(() => {
if (controller.signal.aborted) return;
setSaveMsg(null);
saveTimeoutRef.current = null;
}, 3000);
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setSaveMsg(err instanceof Error ? err.message : "Failed to save schedule");
setSaving(false);
});
@@ -72,6 +80,7 @@ export function BlocklistScheduleSection({ onRunImport, runImportRunning, import
if (saveTimeoutRef.current) {
clearTimeout(saveTimeoutRef.current);
}
submitControllerRef.current?.abort();
};
}, []);

View File

@@ -1,4 +1,4 @@
import { useCallback, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import {
Button,
MessageBar,
@@ -55,6 +55,7 @@ export function BlocklistSourcesSection({ onRunImport, runImportRunning }: Sourc
const [saveError, setSaveError] = useState<string | null>(null);
const [previewOpen, setPreviewOpen] = useState(false);
const [previewSourceItem, setPreviewSourceItem] = useState<BlocklistSource | null>(null);
const submitControllerRef = useRef<AbortController | null>(null);
const openAdd = useCallback((): void => {
setDialogMode("add");
@@ -74,18 +75,26 @@ export function BlocklistSourcesSection({ onRunImport, runImportRunning }: Sourc
const handleSubmit = useCallback(
(values: SourceFormValues): void => {
submitControllerRef.current?.abort();
const controller = new AbortController();
submitControllerRef.current = controller;
setSaving(true);
setSaveError(null);
const op =
dialogMode === "add"
? createSource({ name: values.name, url: values.url, enabled: values.enabled })
: updateSource(editingId ?? -1, { name: values.name, url: values.url, enabled: values.enabled });
op
.then(() => {
if (controller.signal.aborted) return;
setSaving(false);
setDialogOpen(false);
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setSaving(false);
setSaveError(err instanceof Error ? err.message : "Failed to save source");
});
@@ -112,6 +121,12 @@ export function BlocklistSourcesSection({ onRunImport, runImportRunning }: Sourc
setPreviewOpen(true);
}, []);
useEffect(() => {
return (): void => {
submitControllerRef.current?.abort();
};
}, []);
return (
<div className={sectionStyles.section}>
<div className={sectionStyles.sectionHeader}>

View File

@@ -1,4 +1,4 @@
import { useCallback, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import {
Button,
Dialog,
@@ -45,23 +45,37 @@ export function PreviewDialog({ open, source, onClose, fetchPreview }: PreviewDi
const [data, setData] = useState<PreviewResponse | null>(null);
const [loading, setLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
const fetchControllerRef = useRef<AbortController | null>(null);
const handleOpen = useCallback((): void => {
if (!source) return;
fetchControllerRef.current?.abort();
const controller = new AbortController();
fetchControllerRef.current = controller;
setData(null);
setError(null);
setLoading(true);
fetchPreview(source.id)
.then((result) => {
if (controller.signal.aborted) return;
setData(result);
setLoading(false);
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setError(err instanceof Error ? err.message : "Failed to fetch preview");
setLoading(false);
});
}, [source, fetchPreview]);
useEffect(() => {
return (): void => {
fetchControllerRef.current?.abort();
};
}, []);
return (
<Dialog open={open} onOpenChange={(_ev, data) => { if (!data.open) onClose(); }}>
<DialogSurface onAnimationEnd={open ? handleOpen : undefined}>

View File

@@ -5,7 +5,7 @@
* ``POST /api/config/jails/{jail_name}/action`` on confirmation.
*/
import { useCallback, useEffect, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import {
Button,
Checkbox,
@@ -110,6 +110,7 @@ function AssignActionDialogInner({
const [reload, setReload] = useState(false);
const [submitting, setSubmitting] = useState(false);
const [error, setError] = useState<string | null>(null);
const submitControllerRef = useRef<AbortController | null>(null);
const activeJails = jails.filter((j) => j.enabled);
@@ -135,23 +136,37 @@ function AssignActionDialogInner({
if (!actionName || !selectedJail || submitting) return;
const req: AssignActionRequest = { action_name: actionName };
submitControllerRef.current?.abort();
const controller = new AbortController();
submitControllerRef.current = controller;
setSubmitting(true);
setError(null);
onAssign(selectedJail, req, reload)
.then(() => {
if (controller.signal.aborted) return;
onAssigned();
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setError(
err instanceof ApiError ? err.message : "Failed to assign action.",
);
})
.finally(() => {
if (controller.signal.aborted) return;
setSubmitting(false);
});
}, [actionName, selectedJail, reload, submitting, onAssigned, onAssign]);
useEffect(() => {
return (): void => {
submitControllerRef.current?.abort();
};
}, []);
const canConfirm = selectedJail !== "" && !submitting && !jailsLoading;
return (

View File

@@ -5,7 +5,7 @@
* ``POST /api/config/jails/{jail_name}/filter`` on confirmation.
*/
import { useCallback, useEffect, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import {
Button,
Checkbox,
@@ -110,6 +110,7 @@ function AssignFilterDialogInner({
const [reload, setReload] = useState(false);
const [submitting, setSubmitting] = useState(false);
const [error, setError] = useState<string | null>(null);
const submitControllerRef = useRef<AbortController | null>(null);
const activeJails = jails.filter((j) => j.enabled);
@@ -135,23 +136,37 @@ function AssignFilterDialogInner({
if (!filterName || !selectedJail || submitting) return;
const req: AssignFilterRequest = { filter_name: filterName };
submitControllerRef.current?.abort();
const controller = new AbortController();
submitControllerRef.current = controller;
setSubmitting(true);
setError(null);
onAssign(selectedJail, req, reload)
.then(() => {
if (controller.signal.aborted) return;
onAssigned();
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setError(
err instanceof ApiError ? err.message : "Failed to assign filter.",
);
})
.finally(() => {
if (controller.signal.aborted) return;
setSubmitting(false);
});
}, [filterName, selectedJail, reload, submitting, onAssigned, onAssign]);
useEffect(() => {
return (): void => {
submitControllerRef.current?.abort();
};
}, []);
const canConfirm = selectedJail !== "" && !submitting && !jailsLoading;
return (

View File

@@ -6,7 +6,7 @@
* be reused for both filter and action files.
*/
import { useCallback, useEffect, useReducer } from "react";
import { useCallback, useEffect, useReducer, useRef } from "react";
import {
Accordion,
AccordionHeader,
@@ -160,6 +160,7 @@ export function ConfFilesTab({
newContent,
creating,
} = state;
const fetchFileControllerRef = useRef<AbortController | null>(null);
const loadFiles = useCallback(async (signal?: AbortSignal) => {
dispatch({ type: "setLoading", value: true });
@@ -202,8 +203,13 @@ export function ConfFilesTab({
dispatch({ type: "setOpenItems", value: next });
for (const name of newlyOpened) {
if (!Object.prototype.hasOwnProperty.call(contents, name)) {
fetchFileControllerRef.current?.abort();
const controller = new AbortController();
fetchFileControllerRef.current = controller;
void fetchFile(name)
.then((c) => {
if (controller.signal.aborted) return;
dispatch({
type: "setContent",
name,
@@ -216,6 +222,7 @@ export function ConfFilesTab({
});
})
.catch(() => {
if (controller.signal.aborted) return;
dispatch({
type: "setContent",
name,
@@ -233,6 +240,12 @@ export function ConfFilesTab({
[openItems, contents, fetchFile],
);
useEffect(() => {
return (): void => {
fetchFileControllerRef.current?.abort();
};
}, []);
const handleSave = useCallback(
async (name: string) => {
dispatch({ type: "setSaving", value: name });

View File

@@ -5,7 +5,7 @@
* ``POST /api/config/actions`` on confirmation.
*/
import { useCallback, useEffect, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import {
Button,
Dialog,
@@ -73,6 +73,7 @@ export function CreateActionDialog({
const [actionunban, setActionunban] = useState("");
const [submitting, setSubmitting] = useState(false);
const [error, setError] = useState<string | null>(null);
const submitControllerRef = useRef<AbortController | null>(null);
// Reset form when the dialog opens.
useEffect(() => {
@@ -99,23 +100,36 @@ export function CreateActionDialog({
actionunban: actionunban.trim() || null,
};
submitControllerRef.current?.abort();
const controller = new AbortController();
submitControllerRef.current = controller;
setSubmitting(true);
setError(null);
onCreateAction(req)
.then((action) => {
if (controller.signal.aborted) return;
onCreate(action);
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setError(
err instanceof ApiError ? err.message : "Failed to create action.",
);
})
.finally(() => {
if (controller.signal.aborted) return;
setSubmitting(false);
});
}, [name, actionban, actionunban, submitting, onCreate, onCreateAction]);
useEffect(() => {
return (): void => {
submitControllerRef.current?.abort();
};
}, []);
const canConfirm = name.trim() !== "" && !submitting;
return (

View File

@@ -5,7 +5,7 @@
* calls ``POST /api/config/filters`` on confirmation.
*/
import { useCallback, useEffect, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import {
Button,
Dialog,
@@ -82,6 +82,7 @@ export function CreateFilterDialog({
const [ignoreregex, setIgnoreregex] = useState("");
const [submitting, setSubmitting] = useState(false);
const [error, setError] = useState<string | null>(null);
const submitControllerRef = useRef<AbortController | null>(null);
// Reset form when the dialog opens.
useEffect(() => {
@@ -108,23 +109,36 @@ export function CreateFilterDialog({
ignoreregex: splitLines(ignoreregex),
};
submitControllerRef.current?.abort();
const controller = new AbortController();
submitControllerRef.current = controller;
setSubmitting(true);
setError(null);
onCreateFilter(req)
.then((filter) => {
if (controller.signal.aborted) return;
onCreate(filter);
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setError(
err instanceof ApiError ? err.message : "Failed to create filter.",
);
})
.finally(() => {
if (controller.signal.aborted) return;
setSubmitting(false);
});
}, [name, failregex, ignoreregex, submitting, onCreate, onCreateFilter]);
useEffect(() => {
return (): void => {
submitControllerRef.current?.abort();
};
}, []);
const canConfirm = name.trim() !== "" && !submitting;
return (

View File

@@ -5,7 +5,7 @@
* confirmation, seeding the file with a minimal comment header.
*/
import { useCallback, useEffect, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import {
Button,
Dialog,
@@ -63,6 +63,7 @@ export function CreateJailDialog({
const [name, setName] = useState("");
const [submitting, setSubmitting] = useState(false);
const [error, setError] = useState<string | null>(null);
const submitControllerRef = useRef<AbortController | null>(null);
// Reset form when the dialog opens.
useEffect(() => {
@@ -86,23 +87,36 @@ export function CreateJailDialog({
content: `# ${trimmedName}\n`,
};
submitControllerRef.current?.abort();
const controller = new AbortController();
submitControllerRef.current = controller;
setSubmitting(true);
setError(null);
onCreateJail(req)
.then(() => {
if (controller.signal.aborted) return;
onCreated();
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setError(
err instanceof ApiError ? err.message : "Failed to create jail config.",
);
})
.finally(() => {
if (controller.signal.aborted) return;
setSubmitting(false);
});
}, [name, submitting, onCreated, onCreateJail]);
useEffect(() => {
return (): void => {
submitControllerRef.current?.abort();
};
}, []);
const canConfirm = name.trim() !== "" && !submitting;
return (

View File

@@ -725,19 +725,37 @@ function InactiveJailDetail({
const [validating, setValidating] = useState(false);
const [validationResult, setValidationResult] = useState<JailValidationResult | null>(null);
const [validationError, setValidationError] = useState<string | null>(null);
const validateControllerRef = useRef<AbortController | null>(null);
const handleValidate = useCallback((): void => {
validateControllerRef.current?.abort();
const controller = new AbortController();
validateControllerRef.current = controller;
setValidating(true);
setValidationResult(null);
setValidationError(null);
onValidate()
.then((result) => { setValidationResult(result); })
.then((result) => {
if (controller.signal.aborted) return;
setValidationResult(result);
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
handleFetchError(err, createStringErrorAdapter(setValidationError), "Validation request failed.");
})
.finally(() => { setValidating(false); });
.finally(() => {
if (controller.signal.aborted) return;
setValidating(false);
});
}, [onValidate]);
useEffect(() => {
return (): void => {
validateControllerRef.current?.abort();
};
}, []);
const blockingIssues: JailValidationIssue[] =
validationResult?.issues.filter((i) => i.field !== "logpath") ?? [];
const advisoryIssues: JailValidationIssue[] =

View File

@@ -6,7 +6,7 @@
* Feedback is shown via an {@link AutoSaveIndicator}-style message bar.
*/
import { useCallback, useRef, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import {
Accordion,
AccordionHeader,
@@ -97,6 +97,7 @@ export function RawConfigSection({
/** Whether the section has been expanded at least once. */
const loadedRef = useRef(false);
const fetchControllerRef = useRef<AbortController | null>(null);
// --------------------------------------------------------------------------
// Handlers
@@ -109,16 +110,22 @@ export function RawConfigSection({
if (!isOpen || loadedRef.current) return;
loadedRef.current = true;
fetchControllerRef.current?.abort();
const controller = new AbortController();
fetchControllerRef.current = controller;
setFetchLoading(true);
setFetchError(null);
fetchContent()
.then((text) => {
if (controller.signal.aborted) return;
setContent(text);
setLocalText(text);
setFetchLoading(false);
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setFetchError(
err instanceof Error ? err.message : "Failed to load raw content.",
);
@@ -128,6 +135,12 @@ export function RawConfigSection({
[fetchContent],
);
useEffect(() => {
return (): void => {
fetchControllerRef.current?.abort();
};
}, []);
const handleSave = useCallback(async (): Promise<void> => {
setSaveStatus("saving");
setSaveError(null);

View File

@@ -1,4 +1,4 @@
import { useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import {
Badge,
Button,
@@ -35,25 +35,54 @@ export function IgnoreListSection({
const sectionStyles = useCommonSectionStyles();
const [inputVal, setInputVal] = useState("");
const [opError, setOpError] = useState<string | null>(null);
const opControllerRef = useRef<AbortController | null>(null);
const handleAdd = (): void => {
const handleAdd = useCallback((): void => {
if (!inputVal.trim()) return;
opControllerRef.current?.abort();
const controller = new AbortController();
opControllerRef.current = controller;
setOpError(null);
onAdd(inputVal.trim())
.then(() => {
if (controller.signal.aborted) return;
setInputVal("");
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
setOpError(err instanceof Error ? err.message : String(err));
});
};
}, [inputVal, onAdd]);
const handleRemove = useCallback((ip: string): void => {
opControllerRef.current?.abort();
const controller = new AbortController();
opControllerRef.current = controller;
const handleRemove = (ip: string): void => {
setOpError(null);
onRemove(ip).catch((err: unknown) => {
if (controller.signal.aborted) return;
setOpError(err instanceof Error ? err.message : String(err));
});
};
}, [onRemove]);
const handleToggleIgnoreSelf = useCallback((checked: boolean): void => {
opControllerRef.current?.abort();
const controller = new AbortController();
opControllerRef.current = controller;
onToggleIgnoreSelf(checked).catch((err: unknown) => {
if (controller.signal.aborted) return;
setOpError(err instanceof Error ? err.message : String(err));
});
}, [onToggleIgnoreSelf]);
useEffect(() => {
return (): void => {
opControllerRef.current?.abort();
};
}, []);
return (
<div className={sectionStyles.section}>
@@ -70,9 +99,7 @@ export function IgnoreListSection({
label="Ignore self — exclude this server's own IP addresses from banning"
checked={ignoreSelf}
onChange={(_e, data): void => {
onToggleIgnoreSelf(data.checked).catch((err: unknown) => {
setOpError(err instanceof Error ? err.message : String(err));
});
handleToggleIgnoreSelf(data.checked);
}}
/>

View File

@@ -1,4 +1,4 @@
import { useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import { useNavigate } from "react-router-dom";
import {
Badge,
@@ -32,24 +32,40 @@ export function JailInfoSection({ jail, onRefresh, onStart, onStop, onSetIdle, o
const sectionStyles = useCommonSectionStyles();
const navigate = useNavigate();
const [ctrlError, setCtrlError] = useState<string | null>(null);
const controllerRef = useRef<AbortController | null>(null);
const handle =
(fn: () => Promise<unknown>, postNavigate = false) =>
(): void => {
setCtrlError(null);
fn()
.then(() => {
if (postNavigate) {
navigate("/jails");
} else {
onRefresh();
}
})
.catch((err: unknown) => {
const msg = err instanceof Error ? err.message : String(err);
setCtrlError(msg);
});
const handle = useCallback(
(fn: () => Promise<unknown>, postNavigate = false): (() => void) => {
return (): void => {
controllerRef.current?.abort();
const controller = new AbortController();
controllerRef.current = controller;
setCtrlError(null);
fn()
.then(() => {
if (controller.signal.aborted) return;
if (postNavigate) {
navigate("/jails");
} else {
onRefresh();
}
})
.catch((err: unknown) => {
if (controller.signal.aborted) return;
const msg = err instanceof Error ? err.message : String(err);
setCtrlError(msg);
});
};
},
[navigate, onRefresh]
);
useEffect(() => {
return (): void => {
controllerRef.current?.abort();
};
}, []);
return (
<div className={sectionStyles.section}>