Skip to content

fix: report divide-by-zero on mod block instead of Not a Number#7760

Merged
walterbender merged 2 commits into
sugarlabs:masterfrom
rish106-hub:fix/7056-mod-divide-by-zero-message
Jul 9, 2026
Merged

fix: report divide-by-zero on mod block instead of Not a Number#7760
walterbender merged 2 commits into
sugarlabs:masterfrom
rish106-hub:fix/7056-mod-divide-by-zero-message

Conversation

@rish106-hub

Copy link
Copy Markdown
Contributor

Description

The mod block on the Number palette showed the wrong toast when the divisor was zero: Not a Number instead of Cannot divide by zero. (the message the divide block correctly shows for 5 / 0).

Root cause was a token mismatch. MathUtility.doMod threw Error("Division by zero"), but the block-level error dispatcher only recognizes the token "DivByZeroError" (what MathUtility.doDivide throws). The unrecognized message fell through to the default NANERRORMSG toast, so the divide-by-zero case was silently misreported.

Related Issue

This PR fixes #7056

PR Category

  • Bug Fix — Fixes a bug or incorrect behavior
  • Feature — Adds new functionality
  • Performance — Improves performance (load time, memory, rendering, etc.)
  • Tests — Adds or updates test coverage
  • Documentation — Updates to docs, comments, or README
  • Chore / Refactor — Maintenance, cleanup, or refactoring with no behavior change
  • CI/CD — Changes to CI/CD workflows and automation

Changes Made

  • js/utils/mathutils.js: doMod now throws the same error tokens as doDivide"DivByZeroError" (was "Division by zero") for a zero divisor and "NanError" (was "Invalid number input") for non-numeric input. This matches the tokens already documented in doMod's JSDoc and lets any caller dispatch on a single, shared token.
  • js/blocks/NumberBlocks.js: handleMathError now maps DivByZeroError to ZERODIVIDEERRORMSG before falling back to the default. Fixing the shared handler (rather than duplicating the divide block's inline check) means every math block routed through it gets correct divide-by-zero messaging, not just mod.
  • js/utils/__tests__/mathutils.test.js: tightened the existing doMod zero-divisor test to assert the specific DivByZeroError message, and updated the two non-numeric-input assertions to the unified NanError token.

Testing Performed

  • Confirmed the tightened test fails against the old error string and passes with the fix
  • npx jest js/utils/__tests__/mathutils.test.js — all pass
  • npx jest (full suite) — 6075/6075 pass, zero regressions
  • npx eslint and npx prettier --check on all three changed files — clean

Checklist

  • I have tested these changes locally and they work as expected.
  • I have added/updated tests that prove the effectiveness of these changes.
  • I have updated the documentation to reflect these changes, if applicable.
  • I have followed the project's coding style guidelines.
  • I have run `pnpm run lint` and `pnpm exec prettier --check .` with no errors.
  • I have addressed the code review feedback from the previous submission, if applicable.
  • I have enabled "Allow edits from maintainers".

Additional Notes for Reviewers

Single-topic fix. The divide block's behavior is unchanged and served as the reference for the correct mod behavior.

@github-actions github-actions Bot added bug fix Fixes a bug or incorrect behavior tests Adds or updates test coverage size/S Small: 10-49 lines changed area/javascript Changes to JS source files area/tests Changes to test files labels Jul 8, 2026
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.60%. Comparing base (d09973a) to head (00b2277).
⚠️ Report is 48 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7760   +/-   ##
=======================================
  Coverage   54.60%   54.60%           
=======================================
  Files         172      172           
  Lines       57270    57273    +3     
=======================================
+ Hits        31272    31275    +3     
  Misses      25998    25998           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread js/blocks/NumberBlocks.js
@walterbender walterbender merged commit d9fc2a4 into sugarlabs:master Jul 9, 2026
13 checks passed
rakshityadav1868 pushed a commit to rakshityadav1868/musicblocks that referenced this pull request Jul 9, 2026
…rlabs#7760)

* fix: report divide-by-zero on mod block instead of Not a Number

* test: verify mod block reports divide-by-zero error message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files area/tests Changes to test files bug fix Fixes a bug or incorrect behavior size/S Small: 10-49 lines changed tests Adds or updates test coverage

Projects

Development

Successfully merging this pull request may close these issues.

[Bug] Mod block reports "Not a Number" instead of "Cannot divide by zero" when divisor is 0

3 participants