Skip to content

fix(worker): remove unused jsonPaths assignment from createMetacallJsonFiles - #160

Draft
prathamesh04 wants to merge 3 commits into
metacall:masterfrom
prathamesh04:fix/unused-json-paths-assignment
Draft

fix(worker): remove unused jsonPaths assignment from createMetacallJsonFiles#160
prathamesh04 wants to merge 3 commits into
metacall:masterfrom
prathamesh04:fix/unused-json-paths-assignment

Conversation

@prathamesh04

Copy link
Copy Markdown
Contributor

Description

The handleDeployment function was capturing the return value of createMetacallJsonFiles into a jsonPaths variable. However, createMetacallJsonFiles returns Promise<void>, so jsonPaths was always undefined.

This misleading assignment could cause a future developer to accidentally use the wrong jsonPaths variable (shadowed at line 103), leading to silent bugs.

Fixes #133

Changes

  • Await createMetacallJsonFiles without capturing return value
  • Remove unused variable assignment

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Build passes (npm run build)
  • Lint passes (npm run lint)

Fixes infinite loop in assignColorToWorker when all 16 ANSI colors
are allocated. Previously, the do...while loop would spin forever
looking for an unassigned color, blocking the Node.js event loop
and hanging the server at 17+ concurrent deployments.

Changes:
- Replace random retry with deterministic round-robin index
- Remove assignedColorCodes tracking map (no longer needed)
- Remove unused AssignedColorCodesType interface
- Remove stale TODO comments

Colors cycle safely when more than 16 deployments are active.

Fixes metacall#116
The deleteFolder helper in packageUpload was using fs.unlink to remove
the application directory on upload failure. fs.unlink is the POSIX
unlink syscall which only removes a single file inode, not a directory.

When called on a directory path, it fails with EISDIR (Linux/macOS) or
EPERM (Windows), leaving the orphaned directory on disk. Subsequent
upload attempts for the same resource.id may then fail because
ensureFolderExists finds the folder already exists.

Changes:
- Replace fs.unlink with fs.rm({ recursive: true, force: true })
- This correctly removes the entire directory tree

Fixes metacall#119
…onFiles

The handleDeployment function was capturing the return value of
createMetacallJsonFiles into a jsonPaths variable. However,
createMetacallJsonFiles returns Promise<void>, so jsonPaths was always
undefined.

This misleading assignment could cause a future developer to
accidentally use the wrong jsonPaths variable (shadowed at line 103),
leading to silent bugs.

Changes:
- Await createMetacallJsonFiles without capturing return value
- Remove unused variable assignment

Fixes metacall#133
@prathamesh04

Copy link
Copy Markdown
Contributor Author

cc @viferga @josead

@prathamesh04

Copy link
Copy Markdown
Contributor Author

Video Demonstration

I've recorded a video demonstrating PR #160:

What the video shows:

  1. Code walkthrough of the unused variable removal
  2. Build verification
  3. Server startup test
  4. No errors in logs

Key change:

// Before (unused variable - always undefined)
const jsonPaths = await createMetacallJsonFiles(resource.path, resource.jsons);

// After (removed unused assignment)
await createMetacallJsonFiles(resource.path, resource.jsons);

Why this fix matters:

  • createMetacallJsonFiles returns Promise<void>
  • The jsonPaths variable was always undefined
  • Could cause shadowing bugs with the real jsonPaths at line 103

Video file attached below:

cc @viferga @josead

@prathamesh04

prathamesh04 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Video Attached ✅

I've recorded and attached a video demonstrating PR #160.
Screencast from 2026-07-28 19-49-07.webm

Video includes:

  • Code walkthrough showing the fix
  • Test script execution
  • Build verification
  • Server startup test

Summary of the fix:

  • Removed unused jsonPaths variable assignment
  • createMetacallJsonFiles returns Promise<void>, so variable was always undefined
  • Prevents potential shadowing bugs with the real jsonPaths at line 103

cc @viferga @josead

@viferga

viferga commented Aug 3, 2026

Copy link
Copy Markdown
Member

Again you have pushed multiple things in the same PR, I cannot merge this... @prathamesh04

@viferga
viferga marked this pull request as draft August 3, 2026 15:01
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.

Bug: createMetacallJsonFiles return value incorrectly assigned to jsonPaths variable

2 participants