fix: restore BigNumber precision state when decimal.js trig throws precision-limit errors#3682
Open
mvanhorn wants to merge 1 commit into
Open
Conversation
…ecision-limit errors Fixes josdejong#3676
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix at the single choke point where mathjs creates its BigNumber class:
src/type/bignumber/BigNumber.js(the factory that doesDecimal.clone({precision: config.precision})). After cloning, wrap the trig-family prototype methods that use decimal.js's PI-based argument reduction (sin/cos/tan/asin/acos/atanand their cosecant/secant/cotangent/hyperbolic aliases as exposed by decimal.js:sine,cosine,tangent, etc.) in a thin guard that snapshotsBigNumber.precisionandBigNumber.roundingbefore delegating and restores them in afinallyblock, so a thrown DecimalError can no longer leave the class in a corrupted state. In the catch path, rethrow the DecimalError wrapped with an actionable message (e.g.Why this matters
With
math.config({number: 'BigNumber', precision: 509, ...}), evaluatingtan(pi/2)(or any trig call on a high-precision argument) throws[DecimalError] Precision limit exceededfrom decimal.js. The root cause is that decimal.js trig methods internally raise working precision to roughlyprecision + sd(x)guard digits for argument reduction against its PI constant, which is only stored to 1025 digits; 509 + ~509 exceeds that limit. Worse (per the reporter's follow-up comment), decimal.js mutatesCtor.precision/Ctor.roundingbefore computing and only restores them on success, so once the error is thrown the mathjs BigNumber class is left with a corrupted precision (~1025), and every subsequent BigNumber evaluation fails with the same error until the page is reloaded. The hard 1025-digit PI limit lives in decimal.js and cannot be lifted from mathjs, but the sticky global-state corruption and the cryptic error are mathjs-side bugs that can be fixed.See #3676.
Testing
tan(pi/2),sin(0.5),cos(bignumber(1))still return correct BigNumber results and existing trig unit tests pass unchanged. - Sticky-state regression (core fix): configure a fresh math instance with{number: 'BigNumber', precision: 509}; assertevaluate('tan(pi/2)')throws, then assert a subsequent unrelated BigNumber evaluation (e.g.evaluate('1.1 + 2.2')orsin(bignumber(0.5))) succeeds andBigNumber.precisionstill equals 509. - Error path: the thrown error message mentions the precision limit of decimal.js trigonometric functions (not just the bare[DecimalError] Precision limit exceeded), and is an Error instance.Fixes #3676