-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
suppressFormulaErrors is not configurable #1531
Comments
When I correct the statement "$sheet = $excel->getActiveSheet()" to |
I haven't tried the code I posted to be honest :) I'll try to see if I can replicate the issue I have on my project which is much more complex |
I have same trouble. Fix please |
yes, although my code with steps to reproduce is very bad, I believe it's quite obvious from the code that throwing exception that are not catched or triggering errors basically do the same thing : the spreadsheet will not be generated |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
This deserves some attention so I'll send a message again so that stalebot doesn't close this :) |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Did you find a solution to set the property? |
When I used a different formula
That message identifies the worksheet, the cell, and the problem; this certainly gives the developer a good start in correcting the problem. Excel's message, on the other hand, identifies absolutely nothing - good luck fixing the problem now. Another reason I am not sure continuing to generate is a worthwhile action is that the (corrupt) file could be generated in most circumstances anyhow using: $writer->setPreCalculateFormulas(false); At any rate, now that we have a reproducible problem, I can entertain your thoughts on why having the option to continue to generate the spreadsheet is worthwhile. |
@oleibman yes i think the exception is fine in itself (i don't remember it was giving the cell reference which is great!). What I find odd is the suppressFormulaErrors property that is basically inaccessible for all practical purposes and that doesn't really "suppress" anything since the excel file is still invalid.
or if the goal is not to suppress the error, but trigger an error instead of an exception, maybe the property should be called something else like like errorMode : silent, exception, warning, a bit like what PDO is doing. |
Fix PHPOffice#1531. Calculation has a property `suppressFormulaErrors`, which really doesn't work as one might expect. If a calculation throws an exception, the setting of this property might prevent the Exception from being thrown, but it will still trigger an Error. I do not think this makes sense, and will change it so the calculation will return `false`, which is part of the original design but which would essentially never happen. This allows the user to save a corrupt spreadsheet, but this was already possible through the use of `setPreCalculateFormulas(false)` on the Writer, so this doesn't really open any new exposures. It nevertheless might be considered a breaking change because of the difference in behavior. Another break - the visibility of the property is changed from public to private, and a public setter/getter is added. Although I am enabling this ability, I don't necessarily think it's a good idea to make use of it. See the original issue for a discussion of why. It is not mentioned in the official documentation, and I will not be adding documentation for it. The originator discovered it by reading the code, and I think that is sufficient for what will often be an ill-advised choice. Many of the large number (unfortunately not quite all) of problems with Calculation.php in phpstan baseline are addressed. Of note are that private arrays `phpSpreadsheetFunctions`, `controlFunctions`, `comparisonOperators`, and `operatorPrecedence` are changed from static to const.
See PR #3081. I have met you at least part way. It is now configurable, and allows continuation when an exception is thrown in Calculation. It will not attempt to do anything about the cell in error; the result will be |
@oleibman this is looking great |
Fix PHPOffice#1531. This is a replacement for PR PHPOffice#3081 (see last paragraph below), which I will close. Calculation has a property `suppressFormulaErrors`, which really doesn't work as one might expect. If a calculation throws an exception, the setting of this property might prevent the Exception from being thrown, but it will still trigger an Error. I do not think this makes sense, and will change it so the calculation will return `false`, which is part of the original design but which would essentially never happen. This allows the user to save a corrupt spreadsheet, but this was already possible through the use of `setPreCalculateFormulas(false)` on the Writer, so this doesn't really open any new exposures. It nevertheless might be considered a breaking change because of the difference in behavior. Deprecation - the visibility of the existing property is public, which means it can be changed directly. A new private property is added with a public setter/getter. The new property will be used when the existing property is null (default), which will allow the existing property to be deprecated. Function getFunctions is changed to static - the array which it returns is static. Existing callers using it as non-static will still function correctly. Although I am enabling this ability, I don't necessarily think it's a good idea to make use of it. See the original issue for a discussion of why. It is not mentioned in the official documentation, and I will not be adding documentation for it. The originator discovered it by reading the code, and I think that is sufficient for what will often be an ill-advised choice. Many of the large number of problems with Calculation.php in phpstan baseline are addressed. PR 3081 ran afoul of something in phpstan. The changes in this ticket are more limited, adding a number of doc blocks but leaving executable code unchanged.
Had to replace 3082 with 3091 for some reason. Essentially the same change. |
Fix #1531. This is a replacement for PR #3081 (see last paragraph below), which I will close. Calculation has a property `suppressFormulaErrors`, which really doesn't work as one might expect. If a calculation throws an exception, the setting of this property might prevent the Exception from being thrown, but it will still trigger an Error. I do not think this makes sense, and will change it so the calculation will return `false`, which is part of the original design but which would essentially never happen. This allows the user to save a corrupt spreadsheet, but this was already possible through the use of `setPreCalculateFormulas(false)` on the Writer, so this doesn't really open any new exposures. It nevertheless might be considered a breaking change because of the difference in behavior. Deprecation - the visibility of the existing property is public, which means it can be changed directly. A new private property is added with a public setter/getter. The new property will be used when the existing property is null (default), which will allow the existing property to be deprecated. Function getFunctions is changed to static - the array which it returns is static. Existing callers using it as non-static will still function correctly. Although I am enabling this ability, I don't necessarily think it's a good idea to make use of it. See the original issue for a discussion of why. It is not mentioned in the official documentation, and I will not be adding documentation for it. The originator discovered it by reading the code, and I think that is sufficient for what will often be an ill-advised choice. Many of the large number of problems with Calculation.php in phpstan baseline are addressed. PR 3081 ran afoul of something in phpstan. The changes in this ticket are more limited, adding a number of doc blocks but leaving executable code unchanged.
### Added - Extended flag options for the Reader `load()` and Writer `save()` methods - Apply Row/Column limits (1048576 and XFD) in ReferenceHelper [PR #3213](#3213) - Allow the creation of In-Memory Drawings from a string of binary image data, or from a stream. [PR #3157](#3157) - Xlsx Reader support for Pivot Tables [PR #2829](#2829) - Permit Date/Time Entered on Spreadsheet to be calculated as Float [Issue #1416](#1416) [PR #3121](#3121) ### Changed - Nothing ### Deprecated - Direct update of Calculation::suppressFormulaErrors is replaced with setter. - Font public static variable defaultColumnWidths replaced with constant DEFAULT_COLUMN_WIDTHS. - ExcelError public static variable errorCodes replaced with constant ERROR_CODES. - NumberFormat constant FORMAT_DATE_YYYYMMDD2 replaced with existing identical FORMAT_DATE_YYYYMMDD. ### Removed - Nothing ### Fixed - Fixed handling for `_xlws` prefixed functions from Office365 [Issue #3245](#3245) [PR #3247](#3247) - Conditionals formatting rules applied to rows/columns are removed [Issue #3184](#3184) [PR #3213](#3213) - Treat strings containing currency or accounting values as floats in Calculation Engine operations [Issue #3165](#3165) [PR #3189](#3189) - Treat strings containing percentage values as floats in Calculation Engine operations [Issue #3155](#3155) [PR #3156](#3156) and [PR #3164](#3164) - Xlsx Reader Accept Palette of Fewer than 64 Colors [Issue #3093](#3093) [PR #3096](#3096) - Use Locale-Independent Float Conversion for Xlsx Writer Custom Property [Issue #3095](#3095) [PR #3099](#3099) - Allow setting AutoFilter range on a single cell or row [Issue #3102](#3102) [PR #3111](#3111) - Xlsx Reader External Data Validations Flag Missing [Issue #2677](#2677) [PR #3078](#3078) - Reduces extra memory usage on `__destruct()` calls [PR #3092](#3092) - Additional properties for Trendlines [Issue #3011](#3011) [PR #3028](#3028) - Calculation suppressFormulaErrors fix [Issue #1531](#1531) [PR #3092](#3092) - Permit Date/Time Entered on Spreadsheet to be Calculated as Float [Issue #1416](#1416) [PR #3121](#3121) - Incorrect Handling of Data Validation Formula Containing Ampersand [Issue #3145](#3145) [PR #3146](#3146) - Xlsx Namespace Handling of Drawings, RowAndColumnAttributes, MergeCells [Issue #3138](#3138) [PR #3136](#3137) - Generation3 Copy With Image in Footer [Issue #3126](#3126) [PR #3140](#3140) - MATCH Function Problems with Int/Float Compare and Wildcards [Issue #3141](#3141) [PR #3142](#3142) - Fix ODS Read Filter on number-columns-repeated cell [Issue #3148](#3148) [PR #3149](#3149) - Problems Formatting Very Small and Very Large Numbers [Issue #3128](#3128) [PR #3152](#3152) - XlsxWrite preserve line styles for y-axis, not just x-axis [PR #3163](#3163) - Xlsx Namespace Handling of Drawings, RowAndColumnAttributes, MergeCells [Issue #3138](#3138) [PR #3137](#3137) - More Detail for Cyclic Error Messages [Issue #3169](#3169) [PR #3170](#3170) - Improved Documentation for Deprecations - many PRs [Issue #3162](#3162)
This is:
What is the expected behavior?
If a formula is invalid, the behaviour is controlled by Calculation::raiseFormulaError that either throws an exception or triggers an error
This is controlled by the suppressFormulaErrors public variable, but there is no easy way to configure that variable at the spreadsheet level. More over, having a public variable is a bit strange (why not a config option on the spreadsheet with proper getters/setters?). Also the doc blocks of raiseFormulaError are not in line with regular docblocks
It would be nice to actually allow suppressing errors to allow excel generation and not block the process because of one error in a formula (currently, both errors and exception will block the generation of the file)
As an added bonus, it would be nice if the error message could include the actual formula, in large excel file in can be a real mess to find back which formula is causing the error and why it's invalid. having the cell reference is nice, but the actual formula would be super helpful in my opinion.
What is the current behavior?
If a formula contains an error, it will trigger an error. I don't see how to throw an exception, although it wouldn't suppress anything (contrary to what the variable name would let me believe)
If you want to skip errors, you have to comment out everything in the raiseFormulaError method
What are the steps to reproduce?
Just make any sheet with a formula error in it, it will show a Formula Error
Which versions of PhpSpreadsheet and PHP are affected?
1.8.2 to 1.13
The text was updated successfully, but these errors were encountered: