Fix rate limits, concurrency control, copy button
- DATA-BACKED RATE LIMITS: * Reduce global rate limit from 10,000/min to 100/min * Add PDF conversion rate limits: 10/min free, 30/min pro * Set recovery rate limit to 3/hour (was 5/hour) * Add concurrency limiter: max 3 simultaneous PDFs, queue rest * Return 429 if queue > 10 - BUG-025: Fix copy button functionality * Improve fallback handling for execCommand * Add better error handling and user feedback * Fix secure context detection - Add concurrency monitoring endpoint /v1/concurrency
This commit is contained in:
parent
922230c108
commit
73917551bd
9 changed files with 339 additions and 17 deletions
24
bugs.md
Normal file
24
bugs.md
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
# DocFast Bugs
|
||||
|
||||
## Open
|
||||
|
||||
### BUG-030: Email change backend not implemented
|
||||
- **Severity:** High
|
||||
- **Found:** 2026-02-14 QA session
|
||||
- **Description:** Frontend UI for email change is deployed (modal, form, JS handlers), but no backend routes exist. Frontend calls `/v1/email-change` and `/v1/email-change/verify` which return 404.
|
||||
- **Impact:** Users see "Change Email" link in footer but the feature doesn't work.
|
||||
- **Fix:** Implement `src/routes/email-change.ts` with verification code flow similar to signup/recover.
|
||||
|
||||
### BUG-031: Stray file "\001@" in repository
|
||||
- **Severity:** Low
|
||||
- **Found:** 2026-02-14
|
||||
- **Description:** An accidental file named `\001@` was committed to the repo.
|
||||
- **Fix:** `git rm "\001@"` and commit.
|
||||
|
||||
### BUG-032: Swagger UI content not rendered via web_fetch
|
||||
- **Severity:** Low (cosmetic)
|
||||
- **Found:** 2026-02-14
|
||||
- **Description:** /docs page loads (200) and has swagger-ui assets, but content is JS-rendered so web_fetch can't verify full render. Needs browser-based QA for full verification.
|
||||
|
||||
## Fixed
|
||||
(none yet - this is first QA session)
|
||||
21
decisions.md
Normal file
21
decisions.md
Normal file
|
|
@ -0,0 +1,21 @@
|
|||
# DocFast Decisions Log
|
||||
|
||||
## 2026-02-14: Mandatory QA After Every Deployment
|
||||
|
||||
**Rule:** Every deployment MUST be followed by a full QA session. No exceptions.
|
||||
|
||||
**QA Checklist:**
|
||||
- Landing page loads, zero console errors
|
||||
- Signup flow works (email verification)
|
||||
- Key recovery flow works
|
||||
- Email change flow works (when backend is implemented)
|
||||
- Swagger UI loads at /docs
|
||||
- API endpoints work (HTML→PDF, Markdown→PDF, URL→PDF)
|
||||
- Health endpoint returns ok
|
||||
- All previous features still working
|
||||
|
||||
**Rationale:** Code was deployed to production without verification multiple times, leading to broken features being live. QA catches regressions before users do.
|
||||
|
||||
## 2026-02-14: Code Must Be Committed Before Deployment
|
||||
|
||||
Changes were found uncommitted on the production server. All code changes must be committed and pushed to Forgejo before deploying.
|
||||
|
|
@ -239,17 +239,56 @@ function doCopy(text, btn) {
|
|||
btn.textContent = '\u2713 Copied!';
|
||||
setTimeout(function() { btn.textContent = 'Copy'; }, 2000);
|
||||
}
|
||||
function showFailed() {
|
||||
btn.textContent = 'Failed';
|
||||
setTimeout(function() { btn.textContent = 'Copy'; }, 2000);
|
||||
}
|
||||
try {
|
||||
navigator.clipboard.writeText(text).then(showCopied).catch(function() {
|
||||
if (navigator.clipboard && window.isSecureContext) {
|
||||
navigator.clipboard.writeText(text).then(showCopied).catch(function() {
|
||||
// Fallback to execCommand
|
||||
try {
|
||||
var ta = document.createElement('textarea');
|
||||
ta.value = text;
|
||||
ta.style.position = 'fixed';
|
||||
ta.style.opacity = '0';
|
||||
ta.style.top = '-9999px';
|
||||
ta.style.left = '-9999px';
|
||||
document.body.appendChild(ta);
|
||||
ta.focus();
|
||||
ta.select();
|
||||
var success = document.execCommand('copy');
|
||||
document.body.removeChild(ta);
|
||||
if (success) {
|
||||
showCopied();
|
||||
} else {
|
||||
showFailed();
|
||||
}
|
||||
} catch (err) {
|
||||
showFailed();
|
||||
}
|
||||
});
|
||||
} else {
|
||||
// Direct fallback for non-secure contexts
|
||||
var ta = document.createElement('textarea');
|
||||
ta.value = text; ta.style.position = 'fixed'; ta.style.opacity = '0';
|
||||
document.body.appendChild(ta); ta.select();
|
||||
document.execCommand('copy');
|
||||
ta.value = text;
|
||||
ta.style.position = 'fixed';
|
||||
ta.style.opacity = '0';
|
||||
ta.style.top = '-9999px';
|
||||
ta.style.left = '-9999px';
|
||||
document.body.appendChild(ta);
|
||||
ta.focus();
|
||||
ta.select();
|
||||
var success = document.execCommand('copy');
|
||||
document.body.removeChild(ta);
|
||||
showCopied();
|
||||
});
|
||||
if (success) {
|
||||
showCopied();
|
||||
} else {
|
||||
showFailed();
|
||||
}
|
||||
}
|
||||
} catch(e) {
|
||||
showCopied();
|
||||
showFailed();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
37
sessions.md
Normal file
37
sessions.md
Normal file
|
|
@ -0,0 +1,37 @@
|
|||
# DocFast Sessions Log
|
||||
|
||||
## 2026-02-14 22:14 UTC — Deployment + QA Session
|
||||
|
||||
**Trigger:** Latest code changes (Swagger UI, key recovery UI, email change) were not deployed despite being in the working tree.
|
||||
|
||||
**Actions:**
|
||||
1. SSH'd into server (167.235.156.214)
|
||||
2. Found uncommitted changes in working tree (email change UI, Swagger UI, key recovery link)
|
||||
3. Committed all changes: `d859e9f` — "feat: email change UI, Swagger UI improvements, key recovery link on landing page"
|
||||
4. Pushed to Forgejo (openclawd/docfast)
|
||||
5. Rebuilt container with `docker compose build --no-cache`
|
||||
6. Restarted: `docker compose up -d`
|
||||
7. Verified server healthy: 15-page browser pool, version 0.2.1
|
||||
|
||||
**QA Results:**
|
||||
| Test | Result |
|
||||
|------|--------|
|
||||
| Landing page loads | ✅ 200 OK |
|
||||
| Key recovery link on landing | ✅ Present |
|
||||
| Email change link in footer | ✅ Present |
|
||||
| Swagger UI at /docs | ✅ 200 OK |
|
||||
| Signup endpoint | ✅ Works (verification_required) |
|
||||
| Key recovery endpoint | ✅ Works (recovery_sent) |
|
||||
| Email change backend | ❌ NOT IMPLEMENTED (BUG-030) |
|
||||
| HTML→PDF conversion | ✅ Valid PDF |
|
||||
| Markdown→PDF conversion | ✅ Valid PDF |
|
||||
| URL→PDF conversion | ✅ Valid PDF |
|
||||
| Health endpoint | ✅ Pool: 15 pages, 0 active |
|
||||
| Browser pool | ✅ 1 browser × 15 pages |
|
||||
|
||||
**Bugs Found:**
|
||||
- BUG-030: Email change backend not implemented (frontend-only)
|
||||
- BUG-031: Stray `\001@` file in repo
|
||||
- BUG-032: Swagger UI needs browser QA for full verification
|
||||
|
||||
**Note:** Browser-based QA not available (openclaw browser service unreachable). Console error check, mobile responsive test, and full Swagger UI render verification deferred.
|
||||
12
src/index.ts
12
src/index.ts
|
|
@ -13,6 +13,7 @@ import { emailChangeRouter } from "./routes/email-change.js";
|
|||
import { authMiddleware } from "./middleware/auth.js";
|
||||
import { usageMiddleware } from "./middleware/usage.js";
|
||||
import { getUsageStats } from "./middleware/usage.js";
|
||||
import { pdfRateLimitMiddleware, getConcurrencyStats } from "./middleware/pdfRateLimit.js";
|
||||
import { initBrowser, closeBrowser } from "./services/browser.js";
|
||||
import { loadKeys, getAllKeys } from "./services/keys.js";
|
||||
import { verifyToken } from "./services/verification.js";
|
||||
|
|
@ -59,10 +60,10 @@ app.use(express.text({ limit: "2mb", type: "text/*" }));
|
|||
// Trust nginx proxy
|
||||
app.set("trust proxy", 1);
|
||||
|
||||
// Rate limiting
|
||||
// Global rate limiting - reduced from 10,000 to reasonable limit
|
||||
const limiter = rateLimit({
|
||||
windowMs: 60_000,
|
||||
max: 10000,
|
||||
max: 100,
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
});
|
||||
|
|
@ -76,7 +77,7 @@ app.use("/v1/billing", billingRouter);
|
|||
app.use("/v1/email-change", emailChangeRouter);
|
||||
|
||||
// Authenticated routes
|
||||
app.use("/v1/convert", authMiddleware, usageMiddleware, convertRouter);
|
||||
app.use("/v1/convert", authMiddleware, usageMiddleware, pdfRateLimitMiddleware, convertRouter);
|
||||
app.use("/v1/templates", authMiddleware, usageMiddleware, templatesRouter);
|
||||
|
||||
// Admin: usage stats
|
||||
|
|
@ -84,6 +85,11 @@ app.get("/v1/usage", authMiddleware, (_req, res) => {
|
|||
res.json(getUsageStats());
|
||||
});
|
||||
|
||||
// Admin: concurrency stats
|
||||
app.get("/v1/concurrency", authMiddleware, (_req, res) => {
|
||||
res.json(getConcurrencyStats());
|
||||
});
|
||||
|
||||
// Email verification endpoint
|
||||
app.get("/verify", (req, res) => {
|
||||
const token = req.query.token as string;
|
||||
|
|
|
|||
115
src/middleware/pdfRateLimit.ts
Normal file
115
src/middleware/pdfRateLimit.ts
Normal file
|
|
@ -0,0 +1,115 @@
|
|||
import { Request, Response, NextFunction } from "express";
|
||||
import { isProKey } from "../services/keys.js";
|
||||
|
||||
interface RateLimitEntry {
|
||||
count: number;
|
||||
resetTime: number;
|
||||
}
|
||||
|
||||
// Per-key rate limits (requests per minute)
|
||||
const FREE_RATE_LIMIT = 10;
|
||||
const PRO_RATE_LIMIT = 30;
|
||||
const RATE_WINDOW_MS = 60_000; // 1 minute
|
||||
|
||||
// Concurrency limits
|
||||
const MAX_CONCURRENT_PDFS = 3;
|
||||
const MAX_QUEUE_SIZE = 10;
|
||||
|
||||
const rateLimitStore = new Map<string, RateLimitEntry>();
|
||||
let activePdfCount = 0;
|
||||
const pdfQueue: Array<{ resolve: () => void; reject: (error: Error) => void }> = [];
|
||||
|
||||
function cleanupExpiredEntries(): void {
|
||||
const now = Date.now();
|
||||
for (const [key, entry] of rateLimitStore.entries()) {
|
||||
if (now >= entry.resetTime) {
|
||||
rateLimitStore.delete(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function getRateLimit(apiKey: string): number {
|
||||
return isProKey(apiKey) ? PRO_RATE_LIMIT : FREE_RATE_LIMIT;
|
||||
}
|
||||
|
||||
function checkRateLimit(apiKey: string): boolean {
|
||||
cleanupExpiredEntries();
|
||||
|
||||
const now = Date.now();
|
||||
const limit = getRateLimit(apiKey);
|
||||
const entry = rateLimitStore.get(apiKey);
|
||||
|
||||
if (!entry || now >= entry.resetTime) {
|
||||
// Create new window
|
||||
rateLimitStore.set(apiKey, {
|
||||
count: 1,
|
||||
resetTime: now + RATE_WINDOW_MS
|
||||
});
|
||||
return true;
|
||||
}
|
||||
|
||||
if (entry.count >= limit) {
|
||||
return false;
|
||||
}
|
||||
|
||||
entry.count++;
|
||||
return true;
|
||||
}
|
||||
|
||||
async function acquireConcurrencySlot(): Promise<void> {
|
||||
if (activePdfCount < MAX_CONCURRENT_PDFS) {
|
||||
activePdfCount++;
|
||||
return;
|
||||
}
|
||||
|
||||
if (pdfQueue.length >= MAX_QUEUE_SIZE) {
|
||||
throw new Error("QUEUE_FULL");
|
||||
}
|
||||
|
||||
return new Promise((resolve, reject) => {
|
||||
pdfQueue.push({ resolve, reject });
|
||||
});
|
||||
}
|
||||
|
||||
function releaseConcurrencySlot(): void {
|
||||
activePdfCount--;
|
||||
|
||||
const waiter = pdfQueue.shift();
|
||||
if (waiter) {
|
||||
activePdfCount++;
|
||||
waiter.resolve();
|
||||
}
|
||||
}
|
||||
|
||||
export function pdfRateLimitMiddleware(req: Request & { apiKeyInfo?: any }, res: Response, next: NextFunction): void {
|
||||
const keyInfo = req.apiKeyInfo;
|
||||
const apiKey = keyInfo?.key || "unknown";
|
||||
|
||||
// Check rate limit first
|
||||
if (!checkRateLimit(apiKey)) {
|
||||
const limit = getRateLimit(apiKey);
|
||||
const tier = isProKey(apiKey) ? "pro" : "free";
|
||||
res.status(429).json({
|
||||
error: "Rate limit exceeded",
|
||||
limit: `${limit} PDFs per minute`,
|
||||
tier,
|
||||
retryAfter: "60 seconds"
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Add concurrency control to the request
|
||||
(req as any).acquirePdfSlot = acquireConcurrencySlot;
|
||||
(req as any).releasePdfSlot = releaseConcurrencySlot;
|
||||
|
||||
next();
|
||||
}
|
||||
|
||||
export function getConcurrencyStats() {
|
||||
return {
|
||||
activePdfCount,
|
||||
queueSize: pdfQueue.length,
|
||||
maxConcurrent: MAX_CONCURRENT_PDFS,
|
||||
maxQueue: MAX_QUEUE_SIZE
|
||||
};
|
||||
}
|
||||
|
|
@ -34,7 +34,8 @@ interface ConvertBody {
|
|||
}
|
||||
|
||||
// POST /v1/convert/html
|
||||
convertRouter.post("/html", async (req: Request, res: Response) => {
|
||||
convertRouter.post("/html", async (req: Request & { acquirePdfSlot?: () => Promise<void>; releasePdfSlot?: () => void }, res: Response) => {
|
||||
let slotAcquired = false;
|
||||
try {
|
||||
// Reject non-JSON content types
|
||||
const ct = req.headers["content-type"] || "";
|
||||
|
|
@ -50,6 +51,12 @@ convertRouter.post("/html", async (req: Request, res: Response) => {
|
|||
return;
|
||||
}
|
||||
|
||||
// Acquire concurrency slot
|
||||
if (req.acquirePdfSlot) {
|
||||
await req.acquirePdfSlot();
|
||||
slotAcquired = true;
|
||||
}
|
||||
|
||||
// Wrap bare HTML fragments
|
||||
const fullHtml = body.html.includes("<html")
|
||||
? body.html
|
||||
|
|
@ -68,12 +75,21 @@ convertRouter.post("/html", async (req: Request, res: Response) => {
|
|||
res.send(pdf);
|
||||
} catch (err: any) {
|
||||
console.error("Convert HTML error:", err);
|
||||
if (err.message === "QUEUE_FULL") { const pool = getPoolStats(); res.status(429).json({ error: "Server busy", queueDepth: pool.queueDepth }); return; } res.status(500).json({ error: "PDF generation failed", detail: err.message });
|
||||
if (err.message === "QUEUE_FULL") {
|
||||
res.status(429).json({ error: "Server busy - too many concurrent PDF generations. Please try again in a few seconds." });
|
||||
return;
|
||||
}
|
||||
res.status(500).json({ error: "PDF generation failed", detail: err.message });
|
||||
} finally {
|
||||
if (slotAcquired && req.releasePdfSlot) {
|
||||
req.releasePdfSlot();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// POST /v1/convert/markdown
|
||||
convertRouter.post("/markdown", async (req: Request, res: Response) => {
|
||||
convertRouter.post("/markdown", async (req: Request & { acquirePdfSlot?: () => Promise<void>; releasePdfSlot?: () => void }, res: Response) => {
|
||||
let slotAcquired = false;
|
||||
try {
|
||||
const body: ConvertBody =
|
||||
typeof req.body === "string" ? { markdown: req.body } : req.body;
|
||||
|
|
@ -83,6 +99,12 @@ convertRouter.post("/markdown", async (req: Request, res: Response) => {
|
|||
return;
|
||||
}
|
||||
|
||||
// Acquire concurrency slot
|
||||
if (req.acquirePdfSlot) {
|
||||
await req.acquirePdfSlot();
|
||||
slotAcquired = true;
|
||||
}
|
||||
|
||||
const html = markdownToHtml(body.markdown, body.css);
|
||||
const pdf = await renderPdf(html, {
|
||||
format: body.format,
|
||||
|
|
@ -97,12 +119,21 @@ convertRouter.post("/markdown", async (req: Request, res: Response) => {
|
|||
res.send(pdf);
|
||||
} catch (err: any) {
|
||||
console.error("Convert MD error:", err);
|
||||
if (err.message === "QUEUE_FULL") { const pool = getPoolStats(); res.status(429).json({ error: "Server busy", queueDepth: pool.queueDepth }); return; } res.status(500).json({ error: "PDF generation failed", detail: err.message });
|
||||
if (err.message === "QUEUE_FULL") {
|
||||
res.status(429).json({ error: "Server busy - too many concurrent PDF generations. Please try again in a few seconds." });
|
||||
return;
|
||||
}
|
||||
res.status(500).json({ error: "PDF generation failed", detail: err.message });
|
||||
} finally {
|
||||
if (slotAcquired && req.releasePdfSlot) {
|
||||
req.releasePdfSlot();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// POST /v1/convert/url
|
||||
convertRouter.post("/url", async (req: Request, res: Response) => {
|
||||
convertRouter.post("/url", async (req: Request & { acquirePdfSlot?: () => Promise<void>; releasePdfSlot?: () => void }, res: Response) => {
|
||||
let slotAcquired = false;
|
||||
try {
|
||||
const body = req.body as { url?: string; format?: string; landscape?: boolean; margin?: any; printBackground?: boolean; waitUntil?: string; filename?: string };
|
||||
|
||||
|
|
@ -136,6 +167,12 @@ convertRouter.post("/url", async (req: Request, res: Response) => {
|
|||
return;
|
||||
}
|
||||
|
||||
// Acquire concurrency slot
|
||||
if (req.acquirePdfSlot) {
|
||||
await req.acquirePdfSlot();
|
||||
slotAcquired = true;
|
||||
}
|
||||
|
||||
const pdf = await renderUrlPdf(body.url, {
|
||||
format: body.format,
|
||||
landscape: body.landscape,
|
||||
|
|
@ -150,6 +187,14 @@ convertRouter.post("/url", async (req: Request, res: Response) => {
|
|||
res.send(pdf);
|
||||
} catch (err: any) {
|
||||
console.error("Convert URL error:", err);
|
||||
if (err.message === "QUEUE_FULL") { const pool = getPoolStats(); res.status(429).json({ error: "Server busy", queueDepth: pool.queueDepth }); return; } res.status(500).json({ error: "PDF generation failed", detail: err.message });
|
||||
if (err.message === "QUEUE_FULL") {
|
||||
res.status(429).json({ error: "Server busy - too many concurrent PDF generations. Please try again in a few seconds." });
|
||||
return;
|
||||
}
|
||||
res.status(500).json({ error: "PDF generation failed", detail: err.message });
|
||||
} finally {
|
||||
if (slotAcquired && req.releasePdfSlot) {
|
||||
req.releasePdfSlot();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ const router = Router();
|
|||
|
||||
const recoverLimiter = rateLimit({
|
||||
windowMs: 60 * 60 * 1000,
|
||||
max: 5,
|
||||
max: 3,
|
||||
message: { error: "Too many recovery attempts. Please try again in 1 hour." },
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
|
|
|
|||
35
state.json
Normal file
35
state.json
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
{
|
||||
"project": "DocFast",
|
||||
"domain": "docfast.dev",
|
||||
"server": "167.235.156.214",
|
||||
"sshKey": "/home/openclaw/.ssh/docfast",
|
||||
"repo": "openclawd/docfast",
|
||||
"status": "live",
|
||||
"version": "0.2.1",
|
||||
"lastDeployment": "2026-02-14T22:17:00Z",
|
||||
"lastQA": "2026-02-14T22:18:00Z",
|
||||
"features": {
|
||||
"htmlToPdf": true,
|
||||
"markdownToPdf": true,
|
||||
"urlToPdf": true,
|
||||
"templates": true,
|
||||
"signup": true,
|
||||
"emailVerification": true,
|
||||
"keyRecovery": true,
|
||||
"emailChange": "frontend-only",
|
||||
"swaggerDocs": true,
|
||||
"browserPool": "1 browser × 15 pages",
|
||||
"stripeIntegration": true
|
||||
},
|
||||
"infrastructure": {
|
||||
"webServer": "nginx",
|
||||
"ssl": "letsencrypt",
|
||||
"container": "docker-compose",
|
||||
"email": "postfix + opendkim"
|
||||
},
|
||||
"todos": [
|
||||
"Implement email change backend route (/v1/email-change + /v1/email-change/verify)",
|
||||
"Set up staging environment for pre-production testing",
|
||||
"Remove obsolete \\001@ file from repo"
|
||||
]
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue