Skip to content

replace unbounded retry loops with bounded exponential backoff in Turtle and Block cache methods #5855

Open
Ashutoshx7 wants to merge 1 commit intosugarlabs:masterfrom
Ashutoshx7:fix/bounded-retry-and-turtle-tests
Open

replace unbounded retry loops with bounded exponential backoff in Turtle and Block cache methods #5855
Ashutoshx7 wants to merge 1 commit intosugarlabs:masterfrom
Ashutoshx7:fix/bounded-retry-and-turtle-tests

Conversation

@Ashutoshx7
Copy link
Contributor

Summary

This PR fixes critical unbounded retry loops in Turtle._createCache() and Turtle.updateCache() that can cause infinite loops and browser hangs when canvas container bounds are never available. It also consolidates duplicated retry logic across turtle.js and block.js into a shared, well-tested utility.


Problems Fixed

1. Unbounded retry in Turtle._createCache() (infinite loop)

The original code called setTimeout(() => this._createCache(), 200) with no maximum retry limit. If container.getBounds() never returns a value (e.g., due to a rendering failure or detached container), this loops forever, consuming memory and CPU.

2. Broken promise chain in Turtle.updateCache() (silent failures + infinite recursion)

  • The recursive this.updateCache() call was neither awaited nor returned, so failures were silently swallowed
  • No retry limit — infinite recursion if bounds stays null
  • Logged "Block container" instead of "Turtle container" (copy-paste bug)

3. Code duplication across turtle.js and block.js

Three separate hand-rolled retry implementations existed with inconsistent patterns. block.js already had bounded retries with exponential backoff, but turtle.js did not.


Changes

File What changed
js/utils/retryWithBackoff.js New — Shared retry utility with configurable max retries, exponential backoff, and proper Promise semantics
js/turtle.js _createCache(): unbounded setTimeout → bounded retryWithBackoff() (max 20 retries). updateCache(): unbounded async recursion → bounded retry (max 15), fixed log message
js/block.js _createCache() and updateCache(): replaced 60+ lines of hand-rolled retry logic with ~30 lines using shared utility. Restored missing blockBlocks global declaration
js/loader.js Added RequireJS shim entry for utils/retryWithBackoff with dependency declarations for activity/turtle and activity/block
js/activity.js Added "utils/retryWithBackoff" to MYDEFINES array so the module loads at startup
js/tests/retryWithBackoff.test.js New — 19 unit tests covering: immediate success, retry behavior, exponential delay verification, max retries exceeded, error propagation, edge cases

Before / After

Turtle._createCache() — Before

_createCache() {
    this.bounds = this.container.getBounds();
    if (this.bounds == null) {
        setTimeout(() => {
            this._createCache();
        }, 200); // ← No limit. Runs forever.
    } else {
        this.container.cache(this.bounds.x, this.bounds.y, this.bounds.width, this.bounds.height);
    }
}

Turtle._createCache() — After

_createCache() {
    return retryWithBackoff({
        check: () => {
            this.bounds = this.container.getBounds();
            return this.bounds;
        },
        onSuccess: (bounds) => {
            this.container.cache(bounds.x, bounds.y, bounds.width, bounds.height);
        },
        maxRetries: 20,
        initialDelay: 50,
        errorMessage: "Turtle._createCache: could not get container bounds after 20 attempts"
    });
}

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@Ashutoshx7 Ashutoshx7 force-pushed the fix/bounded-retry-and-turtle-tests branch from ab7fa63 to 7bec4d3 Compare February 21, 2026 16:45
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@Ashutoshx7 Ashutoshx7 force-pushed the fix/bounded-retry-and-turtle-tests branch from 7bec4d3 to b4bfb30 Compare February 21, 2026 16:53
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@Ashutoshx7 Ashutoshx7 force-pushed the fix/bounded-retry-and-turtle-tests branch from b4bfb30 to 44deb4c Compare February 21, 2026 17:04
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

Copy link

@shashank03-dev shashank03-dev left a comment

Choose a reason for hiding this comment

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

In retryWithBackoff.js, since you’re already using await, you could simplify the logic by replacing the new Promise wrapper and recursion with a standard await for-loop. It would shorten the code and it would look a bit modern too.

…n Turtle and Block cache methods

- Extract shared retryWithBackoff utility (js/utils/retryWithBackoff.js)
- Fix Turtle._createCache: unbounded setTimeout retry -> bounded (max 20 retries)
- Fix Turtle.updateCache: unbounded async recursion with broken promise chain -> bounded retry
- Fix Turtle.updateCache: incorrect log message ('Block container' -> 'Turtle container')
- Refactor Block._createCache and Block.updateCache to use shared utility
- Register retryWithBackoff in RequireJS loader config (loader.js, activity.js)
- Add 19 unit tests for retryWithBackoff utility
@Ashutoshx7 Ashutoshx7 force-pushed the fix/bounded-retry-and-turtle-tests branch from 44deb4c to 6457434 Compare February 27, 2026 06:09
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

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