-
Notifications
You must be signed in to change notification settings - Fork 4
feat: motd #361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: motd #361
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,36 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { useEffect, useState } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import axios from 'axios'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
interface Motd { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
status: 'up' | 'maintenance'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
message: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export function MaintenanceBanner(): JSX.Element | null { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [motd, setMotd] = useState<Motd | null>(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const fetchMotd = async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const registryUrl = process.env.NEXT_PUBLIC_REGISTRY_URL || 'http://localhost:4321'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const response = await axios.get<Motd>(`${registryUrl}/motd`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setMotd(response.data); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.error('Failed to fetch motd:', error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
void fetchMotd(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, []); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+12
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add timeout and consider periodic refetching. The axios request lacks a timeout configuration, which could cause the request to hang indefinitely if the registry is unresponsive. Additionally, the MOTD is fetched only once on mount, so users won't see status updates without a page refresh. Apply these improvements: useEffect(() => {
const fetchMotd = async () => {
try {
const registryUrl = process.env.NEXT_PUBLIC_REGISTRY_URL || 'http://localhost:4321';
- const response = await axios.get<Motd>(`${registryUrl}/motd`);
+ const response = await axios.get<Motd>(`${registryUrl}/motd`, {
+ timeout: 5000, // 5 second timeout
+ });
setMotd(response.data);
} catch (error) {
console.error('Failed to fetch motd:', error);
}
};
void fetchMotd();
+
+ // Optionally refetch every 5 minutes to catch status changes
+ const interval = setInterval(() => void fetchMotd(), 5 * 60 * 1000);
+ return () => clearInterval(interval);
}, []); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (motd?.status !== 'maintenance') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div className='bg-yellow-500 px-4 py-3 text-center text-sm font-medium text-black'> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
⚠️ {motd.message} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+1
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Eliminate code duplication across platforms. The Consider one of these approaches: Option 1 (Recommended): Extract to a shared package: // packages/ui/src/maintenance-banner.tsx
'use client';
export interface MaintenanceBannerProps {
className?: string;
}
export function MaintenanceBanner({ className = 'bg-yellow-500 px-4 py-3 text-center text-sm font-medium text-black' }: MaintenanceBannerProps) {
// ... shared logic
} Then import in each platform with platform-specific styling if needed. Option 2: If styling must differ significantly, extract just the hook logic: // packages/hooks/src/use-motd.ts
export function useMotd() {
const [motd, setMotd] = useState<Motd | null>(null);
// ... fetch logic
return motd;
} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
'use client'; | ||
|
||
import { useEffect, useState } from 'react'; | ||
import axios from 'axios'; | ||
|
||
interface Motd { | ||
status: 'up' | 'maintenance'; | ||
message: string; | ||
} | ||
|
||
export function MaintenanceBanner() { | ||
const [motd, setMotd] = useState<Motd | null>(null); | ||
|
||
useEffect(() => { | ||
const fetchMotd = async () => { | ||
try { | ||
const registryUrl = process.env.NEXT_PUBLIC_REGISTRY_URL || 'http://localhost:4321'; | ||
const response = await axios.get<Motd>(`${registryUrl}/motd`); | ||
setMotd(response.data); | ||
} catch (error) { | ||
console.error('Failed to fetch motd:', error); | ||
} | ||
}; | ||
|
||
fetchMotd(); | ||
}, []); | ||
|
||
if (motd?.status !== 'maintenance') { | ||
return null; | ||
} | ||
|
||
return ( | ||
<div className="bg-yellow-500 px-4 py-3 text-center text-sm font-medium text-black"> | ||
⚠️ {motd.message} | ||
</div> | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
'use client'; | ||
|
||
import { useEffect, useState } from 'react'; | ||
import axios from 'axios'; | ||
|
||
interface Motd { | ||
status: 'up' | 'maintenance'; | ||
message: string; | ||
} | ||
|
||
export function MaintenanceBanner() { | ||
const [motd, setMotd] = useState<Motd | null>(null); | ||
|
||
useEffect(() => { | ||
const fetchMotd = async () => { | ||
try { | ||
const registryUrl = process.env.NEXT_PUBLIC_REGISTRY_URL || 'http://localhost:4321'; | ||
const response = await axios.get<Motd>(`${registryUrl}/motd`); | ||
setMotd(response.data); | ||
} catch (error) { | ||
console.error('Failed to fetch motd:', error); | ||
} | ||
}; | ||
|
||
fetchMotd(); | ||
}, []); | ||
|
||
if (motd?.status !== 'maintenance') { | ||
return null; | ||
} | ||
|
||
return ( | ||
<div className="bg-yellow-500 px-4 py-3 text-center text-sm font-medium text-black"> | ||
⚠️ {motd.message} | ||
</div> | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,23 @@ | ||||||||||||||||||||||||||||||||||||||||||||
<script lang="ts"> | ||||||||||||||||||||||||||||||||||||||||||||
import { onMount } from 'svelte'; | ||||||||||||||||||||||||||||||||||||||||||||
import axios from 'axios'; | ||||||||||||||||||||||||||||||||||||||||||||
let motd = $state<{ status: 'up' | 'maintenance'; message: string } | null>(null); | ||||||||||||||||||||||||||||||||||||||||||||
onMount(async () => { | ||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||
const registryUrl = import.meta.env.VITE_REGISTRY_URL || 'http://localhost:4321'; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
const response = await axios.get(`${registryUrl}/motd`); | ||||||||||||||||||||||||||||||||||||||||||||
motd = response.data; | ||||||||||||||||||||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||
console.error('Failed to fetch motd:', error); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
8
to
16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add timeout and improve error handling for the MOTD fetch. The axios request lacks a timeout configuration, which could cause the fetch to hang indefinitely if the registry is unresponsive. Additionally, the error handling only logs to console without providing any user feedback or fallback behavior. Apply this diff to add timeout and improve reliability: onMount(async () => {
try {
const registryUrl = import.meta.env.VITE_REGISTRY_URL || 'http://localhost:4321';
- const response = await axios.get(`${registryUrl}/motd`);
+ const response = await axios.get(`${registryUrl}/motd`, {
+ timeout: 5000,
+ });
motd = response.data;
} catch (error) {
- console.error('Failed to fetch motd:', error);
+ console.error('Failed to fetch MOTD:', error);
+ // Silently fail - no banner shown if fetch fails
}
}); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||
</script> | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
{#if motd?.status === 'maintenance'} | ||||||||||||||||||||||||||||||||||||||||||||
<div class="bg-yellow-500 px-4 py-3 text-center text-sm font-medium text-black"> | ||||||||||||||||||||||||||||||||||||||||||||
⚠️ {motd.message} | ||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||
{/if} | ||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,6 +4,7 @@ | |||||||||||||||||||||||||||||||||||||||||
import { onMount } from 'svelte'; | ||||||||||||||||||||||||||||||||||||||||||
import '../app.css'; | ||||||||||||||||||||||||||||||||||||||||||
import { page } from '$app/state'; | ||||||||||||||||||||||||||||||||||||||||||
import MaintenanceBanner from '$lib/fragments/MaintenanceBanner/MaintenanceBanner.svelte'; | ||||||||||||||||||||||||||||||||||||||||||
let { children } = $props(); | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -46,11 +47,14 @@ | |||||||||||||||||||||||||||||||||||||||||
<img src="/images/Logo.svg" alt="logo" /> | ||||||||||||||||||||||||||||||||||||||||||
</main> | ||||||||||||||||||||||||||||||||||||||||||
{:else} | ||||||||||||||||||||||||||||||||||||||||||
<main | ||||||||||||||||||||||||||||||||||||||||||
class="h-[100dvh] overflow-hidden {page.url.pathname.includes('/profile') | ||||||||||||||||||||||||||||||||||||||||||
? 'px-0' | ||||||||||||||||||||||||||||||||||||||||||
: 'px-4'} md:px-0" | ||||||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||||||
{@render children()} | ||||||||||||||||||||||||||||||||||||||||||
</main> | ||||||||||||||||||||||||||||||||||||||||||
<div class="flex h-[100dvh] flex-col overflow-hidden"> | ||||||||||||||||||||||||||||||||||||||||||
<MaintenanceBanner /> | ||||||||||||||||||||||||||||||||||||||||||
<main | ||||||||||||||||||||||||||||||||||||||||||
class="flex-1 overflow-hidden {page.url.pathname.includes('/profile') | ||||||||||||||||||||||||||||||||||||||||||
? 'px-0' | ||||||||||||||||||||||||||||||||||||||||||
: 'px-4'} md:px-0" | ||||||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||||||
{@render children()} | ||||||||||||||||||||||||||||||||||||||||||
</main> | ||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+50
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review overflow handling to ensure content remains scrollable. The wrapper div has Consider:
Apply this diff to allow main content to scroll: - <div class="flex h-[100dvh] flex-col overflow-hidden">
+ <div class="flex h-[100dvh] flex-col">
<MaintenanceBanner />
<main
- class="flex-1 overflow-hidden {page.url.pathname.includes('/profile')
+ class="flex-1 overflow-y-auto {page.url.pathname.includes('/profile')
? 'px-0'
: 'px-4'} md:px-0"
> 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||
{/if} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"status": "maintenance", | ||
"message": "Registry service is down" | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,13 +1,29 @@ | ||||||||||||||||||||||||||||||||||||
import fastify from "fastify"; | ||||||||||||||||||||||||||||||||||||
import { generateEntropy, generatePlatformToken, getJWK } from "./jwt"; | ||||||||||||||||||||||||||||||||||||
import dotenv from "dotenv"; | ||||||||||||||||||||||||||||||||||||
import path from "path"; | ||||||||||||||||||||||||||||||||||||
import path from "node:path"; | ||||||||||||||||||||||||||||||||||||
import { AppDataSource } from "./config/database"; | ||||||||||||||||||||||||||||||||||||
import { VaultService } from "./services/VaultService"; | ||||||||||||||||||||||||||||||||||||
import { UriResolutionService } from "./services/UriResolutionService"; | ||||||||||||||||||||||||||||||||||||
import { KubernetesService } from "./services/KubernetesService"; | ||||||||||||||||||||||||||||||||||||
import cors from "@fastify/cors"; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
import fs from "node:fs"; | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Synchronous I/O blocks the event loop and lacks error handling. The implementation has several serious issues:
Apply this diff to use async I/O with proper error handling: -import fs from "node:fs";
+import fs from "node:fs/promises";
-function loadMotdJSON() {
- const motdJSON = fs.readFileSync(path.resolve(__dirname, "../motd.json"), "utf8");
- return JSON.parse(motdJSON) as {
+async function loadMotdJSON() {
+ try {
+ const motdPath = path.resolve(__dirname, "../motd.json");
+ const motdJSON = await fs.readFile(motdPath, "utf8");
+ return JSON.parse(motdJSON) as {
+ status: "up" | "maintenance"
+ message: string
+ };
+ } catch (error) {
+ server.log.error({ message: "Failed to load MOTD", error });
+ return {
+ status: "up" as const,
+ message: "Registry service is running"
+ };
+ }
+}
+
+let motd = {
- status: "up" | "maintenance"
- message: string
- };
+ status: "up" as const,
+ message: "Registry service is running"
-}
+};
-let motd = loadMotdJSON();
+(async () => {
+ motd = await loadMotdJSON();
+})(); Note: You'll also need to update the file watcher (see next comment). Also applies to: 13-19, 21-21 |
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
function loadMotdJSON() { | ||||||||||||||||||||||||||||||||||||
const motdJSON = fs.readFileSync(path.resolve(__dirname, "../motd.json"), "utf8"); | ||||||||||||||||||||||||||||||||||||
return JSON.parse(motdJSON) as { | ||||||||||||||||||||||||||||||||||||
status: "up" | "maintenance" | ||||||||||||||||||||||||||||||||||||
message: string | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
let motd = loadMotdJSON(); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
fs.watchFile(path.resolve(__dirname, "../motd.json"), (_curr, _prev) => { | ||||||||||||||||||||||||||||||||||||
motd = loadMotdJSON(); | ||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||
Comment on lines
+23
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Major: Use
Apply this diff to use event-based watching with proper cleanup: -fs.watchFile(path.resolve(__dirname, "../motd.json"), (_curr, _prev) => {
- motd = loadMotdJSON();
-});
+const motdWatcher = fs.watch(
+ path.resolve(__dirname, "../motd.json"),
+ async (eventType) => {
+ if (eventType === "change") {
+ motd = await loadMotdJSON();
+ server.log.info("MOTD reloaded");
+ }
+ }
+);
+
+// Cleanup on server close
+server.addHook("onClose", async () => {
+ motdWatcher.close();
+}); Note: This assumes you've converted 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
dotenv.config({ path: path.resolve(__dirname, "../../../.env") }); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const server = fastify({ logger: true }); | ||||||||||||||||||||||||||||||||||||
|
@@ -55,6 +71,10 @@ const checkSharedSecret = async (request: any, reply: any) => { | |||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
server.get("/motd", async (request, reply) => { | ||||||||||||||||||||||||||||||||||||
return motd; | ||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
// Create a new vault entry | ||||||||||||||||||||||||||||||||||||
server.post( | ||||||||||||||||||||||||||||||||||||
"/register", | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Extract shared type definition to prevent drift.
The
Motd
interface is duplicated across three platforms (blabsy, eVoting, group-charter-manager). This creates a maintenance burden and risk of type inconsistencies.Consider creating a shared types package (e.g.,
packages/types/src/motd.ts
) and importing from there:Then import in each platform:
🤖 Prompt for AI Agents