Skip to content

fix: release running task after error so more tasks can run#7031

Open
UberMouse wants to merge 2 commits intomaplibre:mainfrom
UberMouse:fix-task-already-running-after-crash
Open

fix: release running task after error so more tasks can run#7031
UberMouse wants to merge 2 commits intomaplibre:mainfrom
UberMouse:fix-task-already-running-after-crash

Conversation

@UberMouse
Copy link
Copy Markdown

@UberMouse UberMouse commented Jan 30, 2026

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

Does the proposed fix described in #6093
When a task errors, set this._currentlyRunning to false instead of leaving it as true to allow the next task to run instead of throwing an error.

@UberMouse UberMouse force-pushed the fix-task-already-running-after-crash branch from f4334b9 to b579f42 Compare January 30, 2026 03:18
if (task.cancelled) continue;
task.callback(timeStamp);
if (this._cleared) break;
try {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't the task execution be wrapped in try catch and not the entire for loop?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this ensures that exiting the function always releases the _currentlyRunning lock

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What will happen if I add two tasks - the first that throws and the second that should pass.
And then call the run method? Will the second task run or be "ignored"?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Don't know. Don't have any idea how this Task system works, my boss just complained about this error so I found the linked issue and did a PR for the solution they suggested and never bothered to PR. Hopefully someone who knows anything about this Task system can check it out

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would recommend trying to take a deeper look then, otherwise I'll have a hard time merging this as is...

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.51%. Comparing base (00f3dc2) to head (6c63636).
⚠️ Report is 174 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7031   +/-   ##
=======================================
  Coverage   92.51%   92.51%           
=======================================
  Files         288      288           
  Lines       23925    23926    +1     
  Branches     5081     5081           
=======================================
+ Hits        22134    22135    +1     
  Misses       1791     1791           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@HarelM
Copy link
Copy Markdown
Collaborator

HarelM commented Mar 20, 2026

Any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants