Skip to content

Refactor: Remove redundant logic and improve code clarity in worker.ts#22

Merged
nota9x merged 5 commits intomainfrom
copilot/remove-redundant-304-check
Feb 11, 2026
Merged

Refactor: Remove redundant logic and improve code clarity in worker.ts#22
nota9x merged 5 commits intomainfrom
copilot/remove-redundant-304-check

Conversation

Copy link
Contributor

Copilot AI commented Feb 11, 2026

Pull Request (Website)

1. Type of Change:
What does this PR do? (Check one)

  • Bug Fix (e.g., Fixing a broken tool or layout issue)
  • New Feature (e.g., Adding a new tool or page)
  • Content Update (e.g., New blog post or text changes)
  • Code Refactor (e.g., Cleaning up TypeScript or CSS)

2. Related Issue:
Does this fix an existing issue? Please link it here (e.g., "Closes #24").

N/A

3. Summary of Changes:
Briefly explain what you changed and why. (e.g., "Fixed mobile navbar not opening on iOS," "Added a new blog post about Securly SSL decryption")

Applied code quality improvements and bug fixes to src/worker.ts:

  • Removed redundant 304 handling: handleAssetResponse() already handles 304 responses, eliminated duplicate check for notFoundResponse
  • Fixed 404 response handling bug: Corrected logic to only short-circuit for 304 Not Modified responses, ensuring 200 OK responses from /404.html are properly rewrapped with status 404 (not returned with status 200)
  • Refactored CSP definition: Split single-line string into multi-line array for maintainability—output identical to original
  • Applied Prettier formatting: Added trailing comma to CSP directives array per repo conventions
  • Improved variable naming: handledMappedmappedAssetResponse, handledStaticstaticAssetResponse, handledNotFoundprocessedNotFoundResponse

Before:

if (notFoundResponse.status === 304) {
  const handledNotFound = handleAssetResponse(notFoundResponse);
  if (handledNotFound) return handledNotFound;
}

After:

const processedNotFoundResponse = handleAssetResponse(notFoundResponse);
// Only short-circuit 304 Not Modified responses; allow 200 OK to be rewrapped as 404 below.
if (processedNotFoundResponse && processedNotFoundResponse.status === 304) {
  return processedNotFoundResponse;
}

4. Verification & Testing:
How did you verify these changes?

  • I have built the worker locally (npm run build) without errors.
  • I have tested the changes on a local development server (npm run dev).
  • (If UI related) I have checked how this looks on mobile/Chromebooks.

5. Screenshots (Optional):
If this is a visual change, please paste a screenshot or GIF below to help us review it faster.

N/A


Final Checklist:

  • I have read the Contributing Guidelines.
  • My code follows the project's style (clean TypeScript, proper asset placement).
Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The 304 status check for notFoundResponse is redundant since handleAssetResponse already handles 304 responses. This duplicates the logic from the handleAssetResponse function.","fixFiles":[{"filePath":"src/worker.ts","diff":"diff --git a/src/worker.ts b/src/worker.ts\n--- a/src/worker.ts\n+++ b/src/worker.ts\n@@ -139,13 +139,9 @@\n       headers: request.headers,\n     });\n \n-    // Handle 304 responses using the helper\n-    if (notFoundResponse.status === 304) {\n-      const handledNotFound = handleAssetResponse(notFoundResponse);\n-      if (handledNotFound) {\n-        return handledNotFound;\n-      }\n-      // If the helper returns null, fall through to the 404 handling below.\n+    const handledNotFound = handleAssetResponse(notFoundResponse);\n+    if (handledNotFound) {\n+      return handledNotFound;\n     }\n     \n     // For successful responses, wrap in 404 status\n"}]},{"message":"[nitpick] The Content Security Policy is defined as a single long string making it difficult to read and maintain. Consider breaking it into a multi-line template literal or defining it as an array of directives that get joined.","fixFiles":[{"filePath":"src/worker.ts","diff":"diff --git a/src/worker.ts b/src/worker.ts\n--- a/src/worker.ts\n+++ b/src/worker.ts\n@@ -43,10 +43,18 @@\n     headers.set('Referrer-Policy', 'strict-origin-when-cross-origin');\n   if (!headers.has('X-Frame-Options')) headers.set('X-Frame-Options', 'SAMEORIGIN');\n   if (!headers.has('Content-Security-Policy')) {\n-    headers.set(\n-      'Content-Security-Policy',\n-      \"default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com; script-src 'self'; connect-src 'self' https://dns-monitor.a9x.workers.dev https://raw.githubusercontent.com; object-src 'none'; base-uri 'self'; frame-ancestors 'self'\"\n-    );\n+    const cspDirectives = [\n+      \"default-src 'self';\",\n+      \"img-src 'self' data:;\",\n+      \"style-src 'self' 'unsafe-inline' https://fonts.googleapis.com;\",\n+      \"font-src 'self' https://fonts.gstatic.com;\",\n+      \"script-src 'self';\",\n+      \"connect-src 'self' https://dns-monitor.a9x.workers.dev https://raw.githubusercontent.com;\",\n+      \"object-src 'none';\",\n+      \"base-uri 'self';\",\n+      \"frame-ancestors 'self'\"\n+    ];\n+    headers.set('Content-Security-Policy', cspDirectives.join(' '));\n   }\n   if (!headers.has('Strict-Transport-Security')) {\n     headers.set('Strict-Transport-Security', 'max-age=31536000; includeSubDomains; preload');\n"}]},{"message":"[nitpick] The variable name 'handledMapped' is unclear. Consider renaming to 'mappedAssetResponse' or 'processedResponse' to better indicate what it represents.","fixFiles":[{"filePath":"src/worker.ts","diff":"diff --git a/src/worker.ts b/src/worker.ts\n--- a/src/worker.ts\n+++ b/src/worker.ts\n@@ -120,8 +120,8 @@\n         method: request.method,\n         headers: request.headers,\n       });\n-      const handledMapped = handleAssetResponse(response);\n-      if (handledMapped) return handledMapped;\n+      const mappedAssetResponse = handleAssetResponse(response);\n+      if (mappedAssetResponse) return mappedAssetResponse;\n       // For mapped HTML routes, if the asset fetch fails, fall through to 404 handling below.\n     } else {\n       // For static assets (CSS, JS, images, etc.), pass through\n"}]},{"message":"[nitpick] The variable name 'handledStatic' is unclear. Consider renaming to 'staticAssetResponse' or 'processedResponse' to better indicate what it represents.","fixFiles":[{"filePath":"src/worker.ts","diff":"diff --git a/src/worker.ts b/src/worker.ts\n--- a/src/worker.ts\n+++ b/src/worker.ts\n@@ -126,8 +126,8 @@\n     } else {\n       // For static assets (CSS, JS, images, etc.), pass through\n       const response = await env.ASSETS.fetch(request);\n-      const handledStatic = handleAssetResponse(response);\n-      if (handledStatic) return handledStatic;\n+      const staticAssetResponse = handleAssetResponse(response);\n+      if (staticAssetResponse) return staticAssetResponse;\n     }\n \n     // 404 fallback - fetch 404.html\n"}]},{"message":"[nitpick] The variable name 'handledNotFound' is unclear. Consider renaming to 'processedNotFoundResponse' to better indicate what it represents.","fixFiles":[{"filePath":"src/worker.ts","diff":"diff --git a/src/worker.ts b/src/worker.ts\n--- a/src/worker.ts\n+++ b/src/worker.ts\n@@ -141,9 +141,9 @@\n \n     // Handle 304 responses using the helper\n     if (notFoundResponse.status === 304) {\n-      const handledNotFound = handleAssetResponse(notFoundResponse);\n-      if (handledNotFound) {\n-        return handledNotFound;\n+      const processedNotFoundResponse = handleAssetResponse(notFoundResponse);...

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: nota9x <105396031+nota9x@users.noreply.github.com>
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 11, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
hapara-fail e2b213e Commit Preview URL

Branch Preview URL
Feb 11 2026, 02:18 AM

Copilot AI and others added 2 commits February 11, 2026 02:03
Co-authored-by: nota9x <105396031+nota9x@users.noreply.github.com>
Co-authored-by: nota9x <105396031+nota9x@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove redundant 304 status check in response handling Refactor: Remove redundant logic and improve code clarity in worker.ts Feb 11, 2026
Copilot AI requested a review from nota9x February 11, 2026 02:06
@nota9x nota9x marked this pull request as ready for review February 11, 2026 02:12
Copilot AI review requested due to automatic review settings February 11, 2026 02:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors src/worker.ts to reduce redundant response-handling logic and improve readability around CSP definition and intermediate variable naming in the Cloudflare Worker entrypoint.

Changes:

  • Refactors the Content Security Policy definition into a directive array joined into a single header value.
  • Renames intermediate variables to better reflect what handleAssetResponse() returns.
  • Simplifies 404 fallback handling by attempting to process the /404.html response via the shared helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/worker.ts Outdated
}
// If the helper returns null, fall through to the 404 handling below.
const processedNotFoundResponse = handleAssetResponse(notFoundResponse);
if (processedNotFoundResponse) {
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: handleAssetResponse() returns a non-null response for any ok response. In the 404 fallback flow, this causes /404.html to be returned with its original status (typically 200) instead of being wrapped as a 404. Previously only 304s were short-circuited; successful 404.html responses were intentionally rewrapped with status: 404 below.

Fix: only short-circuit for notFoundResponse.status === 304 (or apply security headers without returning early for ok), so the notFoundResponse.ok branch can still wrap the body with a 404 status.

Suggested change
if (processedNotFoundResponse) {
// Only short-circuit 304 Not Modified responses; allow 200 OK to be rewrapped as 404 below.
if (processedNotFoundResponse && processedNotFoundResponse.status === 304) {

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e2b213e. Now only short-circuits for 304 responses, allowing 200 OK responses from /404.html to be properly rewrapped with status 404.

Comment on lines +46 to +50
const cspDirectives = [
"default-src 'self';",
"img-src 'self' data:;",
"style-src 'self' 'unsafe-inline' https://fonts.googleapis.com;",
"font-src 'self' https://fonts.gstatic.com;",
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting/convention: .prettierrc sets singleQuote: true and trailingComma: "es5", but this new cspDirectives array uses double-quoted strings and omits the trailing comma on the last element. Please run Prettier (or update these literals) so this block matches the repo’s formatting rules and avoids churn in future diffs.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e2b213e. Applied Prettier formatting - added trailing comma to CSP directives array per repo conventions.

@nota9x
Copy link
Member

nota9x commented Feb 11, 2026

@copilot apply changes based on the comments in this thread

Co-authored-by: nota9x <105396031+nota9x@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 11, 2026

@copilot apply changes based on the comments in this thread

Changes applied in e2b213e:

  1. Fixed 404 response handling to only short-circuit for 304 responses
  2. Applied Prettier formatting with trailing comma to CSP directives

@nota9x nota9x merged commit 0a63821 into main Feb 11, 2026
4 checks passed
@nota9x nota9x deleted the copilot/remove-redundant-304-check branch February 11, 2026 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants