JsStepDebuggerService.runUntil() (used by stepInto, stepOver, and stepOut) is a fully synchronous while loop bounded only by STEP_BUDGET = 50_000 micro-steps. While the budget prevents the truly-pathological hang, a single "Step Over" across a function that calls arr.forEach(...) on a sizeable array can block the main thread for 100–500ms with no visual feedback. The user clicks once and the tab “hangs” — in an educational sandbox that reads as a broken tool.
Contrast: the tick() run loop already chunks into batches of STEPS_PER_TICK = 200 and yields via setTimeout(0) between batches, so long Run sessions stay responsive. The step-action path skipped that pattern presumably for simplicity.
The public surface (stepInto/Over/Out) becomes fire-and-forget instead of synchronous, but already-async UI consumers don't care; the only adjustment is in tests that assert state synchronously after a step call.
Suggested fix (deferred / cheaper)
Lower STEP_BUDGET to ~5,000 and surface "Step took too long; click Step again to continue" in the chip. Less invasive, uglier UX, but unblocks shipping.
Owner pointers
libraries/tools/src/services/js-step-debugger/js-step-debugger.service.ts — runUntil() lines 661–707, tick() lines 596–652 for the chunk-yield pattern to mirror.
Tests live at services/js-step-debugger/js-step-debugger.service.spec.ts and currently invoke stepInto/Over/Out synchronously — they’ll need await after the migration.
Related
Found during code review of the Monaco migration + JS debugger feature wave.
## Problem
`JsStepDebuggerService.runUntil()` (used by `stepInto`, `stepOver`, and `stepOut`) is a fully synchronous `while` loop bounded only by `STEP_BUDGET = 50_000` micro-steps. While the budget prevents the truly-pathological hang, a single "Step Over" across a function that calls `arr.forEach(...)` on a sizeable array can block the main thread for 100–500ms with no visual feedback. The user clicks once and the tab “hangs” — in an educational sandbox that reads as a broken tool.
Contrast: the `tick()` run loop already chunks into batches of `STEPS_PER_TICK = 200` and yields via `setTimeout(0)` between batches, so long Run sessions stay responsive. The step-action path skipped that pattern presumably for simplicity.
## Repro
1. Open `/tools/javascript/lessons` in the playground.
2. Paste:
```js
function spin(n) {
var s = 0;
for (var i = 0; i < n; i++) s += i;
return s;
}
spin(20000);
```
3. Set a breakpoint on the `spin(20000)` line, click **Step Into** to descend, then click **Step Over** on the `spin(20000)` invocation.
4. Observe the tab is unresponsive for ~hundreds of ms; the status chip never visibly updates to “Running…”.
## Suggested fix (cheap)
Make `runUntil` iterative-with-yield:
- Run an inner batch of `STEPS_PER_TICK` micro-steps.
- After each batch, schedule the next batch via `setTimeout(0)` and return.
- Stop conditions (predicate / breakpoint / completion / budget) stay the same.
The public surface (`stepInto/Over/Out`) becomes fire-and-forget instead of synchronous, but already-async UI consumers don't care; the only adjustment is in tests that assert state synchronously after a step call.
## Suggested fix (deferred / cheaper)
Lower `STEP_BUDGET` to ~5,000 and surface "Step took too long; click Step again to continue" in the chip. Less invasive, uglier UX, but unblocks shipping.
## Owner pointers
- `libraries/tools/src/services/js-step-debugger/js-step-debugger.service.ts` — `runUntil()` lines 661–707, `tick()` lines 596–652 for the chunk-yield pattern to mirror.
- Tests live at `services/js-step-debugger/js-step-debugger.service.spec.ts` and currently invoke `stepInto/Over/Out` synchronously — they’ll need `await` after the migration.
## Related
Found during code review of the Monaco migration + JS debugger feature wave.
libraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts — went with the cheap fix from the ticket. runUntil(predicate) now mirrors tick()'s chunked-yield pattern: an inner batch of STEPS_PER_TICK = 200 micro-steps, then re-enter via setTimeout(0). STEP_BUDGET = 50_000 is now enforced across the whole call rather than spinning a synchronous while, so a Step Over across spin(20000) (the repro) never blocks the main thread for more than a single batch (~1–2ms). _state.set("running") happens synchronously at entry so the toolbar status chip flips to “Running…” the moment the user clicks — that's the user-visible payoff.
API surface — stepInto / stepOver / stepOut now return Promise<void>. Fire-and-forget callers (Angular templates) need no change; consumers driving multiple steps in a loop must await.
Cancellation — added a private runUntilResolver field plus a finishRunUntil() helper. pause(), stop(), and prepare() all drain it, so an await stepInto() parked between batches resolves cleanly when the user clicks Pause instead of dangling forever. The pause-mid-step interrupt is the second user-visible win (used to be a no-op because the synchronous loop never gave the event loop a chance to deliver the click).
libraries/features/dev-tools-blockly/src/lib/interpreters/javascript-interpreter.service.ts — the only consumer that drove the loop synchronously. Its step() method's inner for loop now awaits this.core.stepInto() before reading state(), otherwise the loop would break on the first iteration with state still "running".
Tests
Migrated js-step-debugger.service.spec.ts — every stepInto/stepOver/stepOut is now awaited and the wrapping it() callbacks are async, exactly as the owner pointers in the ticket warned.
New describe("issue #74 -- step actions yield to the browser") block with three regressions:
stepInto() returns a Promise (API contract guard).
Step Over across spin(2000) (~12k internal micro-steps, ≫ STEPS_PER_TICK) settles in "paused" on the next line without exhausting the budget.
pause() mid-step resolves the awaiter to "paused" — pre-fix this would hang.
All 49 tests pass (46 prior + 3 new). Typecheck clean on platform-js-step-debugger, feature-dev-tools-blockly, and feature-dev-tools-javascript-runner.
Note on stale paths
The owner pointers in the ticket cited libraries/tools/src/services/js-step-debugger/...; the service has since been extracted into its own library at libraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts. Line numbers had also drifted by ~8 (runUntil 669–715, tick 604–660 pre-fix). Structure was unchanged so the fix dropped in cleanly.
Closing.
Fixed.
## Summary of changes
**`libraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts`** — went with the cheap fix from the ticket. `runUntil(predicate)` now mirrors `tick()`'s chunked-yield pattern: an inner batch of `STEPS_PER_TICK = 200` micro-steps, then re-enter via `setTimeout(0)`. `STEP_BUDGET = 50_000` is now enforced across the whole call rather than spinning a synchronous `while`, so a Step Over across `spin(20000)` (the repro) never blocks the main thread for more than a single batch (~1–2ms). `_state.set("running")` happens synchronously at entry so the toolbar status chip flips to “Running…” the moment the user clicks — that's the user-visible payoff.
**API surface** — `stepInto` / `stepOver` / `stepOut` now return `Promise<void>`. Fire-and-forget callers (Angular templates) need no change; consumers driving multiple steps in a loop must `await`.
**Cancellation** — added a private `runUntilResolver` field plus a `finishRunUntil()` helper. `pause()`, `stop()`, and `prepare()` all drain it, so an `await stepInto()` parked between batches resolves cleanly when the user clicks Pause instead of dangling forever. The pause-mid-step interrupt is the second user-visible win (used to be a no-op because the synchronous loop never gave the event loop a chance to deliver the click).
**`libraries/features/dev-tools-blockly/src/lib/interpreters/javascript-interpreter.service.ts`** — the only consumer that drove the loop synchronously. Its `step()` method's inner `for` loop now `await`s `this.core.stepInto()` before reading `state()`, otherwise the loop would break on the first iteration with state still `"running"`.
## Tests
- Migrated `js-step-debugger.service.spec.ts` — every `stepInto/stepOver/stepOut` is now `await`ed and the wrapping `it()` callbacks are `async`, exactly as the owner pointers in the ticket warned.
- New `describe("issue #74 -- step actions yield to the browser")` block with three regressions:
1. `stepInto()` returns a `Promise` (API contract guard).
2. Step Over across `spin(2000)` (~12k internal micro-steps, ≫ `STEPS_PER_TICK`) settles in `"paused"` on the next line without exhausting the budget.
3. `pause()` mid-step resolves the awaiter to `"paused"` — pre-fix this would hang.
- All 49 tests pass (46 prior + 3 new). Typecheck clean on `platform-js-step-debugger`, `feature-dev-tools-blockly`, and `feature-dev-tools-javascript-runner`.
## Note on stale paths
The owner pointers in the ticket cited `libraries/tools/src/services/js-step-debugger/...`; the service has since been extracted into its own library at `libraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts`. Line numbers had also drifted by ~8 (`runUntil` 669–715, `tick` 604–660 pre-fix). Structure was unchanged so the fix dropped in cleanly.
Closing.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Problem
JsStepDebuggerService.runUntil()(used bystepInto,stepOver, andstepOut) is a fully synchronouswhileloop bounded only bySTEP_BUDGET = 50_000micro-steps. While the budget prevents the truly-pathological hang, a single "Step Over" across a function that callsarr.forEach(...)on a sizeable array can block the main thread for 100–500ms with no visual feedback. The user clicks once and the tab “hangs” — in an educational sandbox that reads as a broken tool.Contrast: the
tick()run loop already chunks into batches ofSTEPS_PER_TICK = 200and yields viasetTimeout(0)between batches, so long Run sessions stay responsive. The step-action path skipped that pattern presumably for simplicity.Repro
/tools/javascript/lessonsin the playground.spin(20000)line, click Step Into to descend, then click Step Over on thespin(20000)invocation.Suggested fix (cheap)
Make
runUntiliterative-with-yield:STEPS_PER_TICKmicro-steps.setTimeout(0)and return.The public surface (
stepInto/Over/Out) becomes fire-and-forget instead of synchronous, but already-async UI consumers don't care; the only adjustment is in tests that assert state synchronously after a step call.Suggested fix (deferred / cheaper)
Lower
STEP_BUDGETto ~5,000 and surface "Step took too long; click Step again to continue" in the chip. Less invasive, uglier UX, but unblocks shipping.Owner pointers
libraries/tools/src/services/js-step-debugger/js-step-debugger.service.ts—runUntil()lines 661–707,tick()lines 596–652 for the chunk-yield pattern to mirror.services/js-step-debugger/js-step-debugger.service.spec.tsand currently invokestepInto/Over/Outsynchronously — they’ll needawaitafter the migration.Related
Found during code review of the Monaco migration + JS debugger feature wave.
Fixed.
Summary of changes
libraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts— went with the cheap fix from the ticket.runUntil(predicate)now mirrorstick()'s chunked-yield pattern: an inner batch ofSTEPS_PER_TICK = 200micro-steps, then re-enter viasetTimeout(0).STEP_BUDGET = 50_000is now enforced across the whole call rather than spinning a synchronouswhile, so a Step Over acrossspin(20000)(the repro) never blocks the main thread for more than a single batch (~1–2ms)._state.set("running")happens synchronously at entry so the toolbar status chip flips to “Running…” the moment the user clicks — that's the user-visible payoff.API surface —
stepInto/stepOver/stepOutnow returnPromise<void>. Fire-and-forget callers (Angular templates) need no change; consumers driving multiple steps in a loop mustawait.Cancellation — added a private
runUntilResolverfield plus afinishRunUntil()helper.pause(),stop(), andprepare()all drain it, so anawait stepInto()parked between batches resolves cleanly when the user clicks Pause instead of dangling forever. The pause-mid-step interrupt is the second user-visible win (used to be a no-op because the synchronous loop never gave the event loop a chance to deliver the click).libraries/features/dev-tools-blockly/src/lib/interpreters/javascript-interpreter.service.ts— the only consumer that drove the loop synchronously. Itsstep()method's innerforloop nowawaitsthis.core.stepInto()before readingstate(), otherwise the loop would break on the first iteration with state still"running".Tests
js-step-debugger.service.spec.ts— everystepInto/stepOver/stepOutis nowawaited and the wrappingit()callbacks areasync, exactly as the owner pointers in the ticket warned.describe("issue #74 -- step actions yield to the browser")block with three regressions:stepInto()returns aPromise(API contract guard).spin(2000)(~12k internal micro-steps, ≫STEPS_PER_TICK) settles in"paused"on the next line without exhausting the budget.pause()mid-step resolves the awaiter to"paused"— pre-fix this would hang.platform-js-step-debugger,feature-dev-tools-blockly, andfeature-dev-tools-javascript-runner.Note on stale paths
The owner pointers in the ticket cited
libraries/tools/src/services/js-step-debugger/...; the service has since been extracted into its own library atlibraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts. Line numbers had also drifted by ~8 (runUntil669–715,tick604–660 pre-fix). Structure was unchanged so the fix dropped in cleanly.Closing.