suppressFormulaErrors is not configurable

[lekoala]

This is:

- [ ] a bug report
- [x] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

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

// trigger an error, but nicely, if need be
protected function raiseFormulaError($errorMessage)

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

// trigger an error, but nicely, if need be
protected function raiseFormulaError($errorMessage)
{
    $this->formulaError = $errorMessage;
    $this->cyclicReferenceStack->clear();
    // if (!$this->suppressFormulaErrors) {
        // throw new Exception($errorMessage);
    // }
    // trigger_error($errorMessage, E_USER_ERROR);

    return false;
}

What are the steps to reproduce?

Just make any sheet with a formula error in it, it will show a Formula Error

<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
// add code that show the issue here...
$sheet->setCellValueByColumnAndRow(1, 1, '=SOMEERRORFUNCTION(A1:A10)');

$writer = IOFactory::createWriter($spreadsheet, 'Xlsx');
$writer->save('php://output');

Which versions of PhpSpreadsheet and PHP are affected?

1.8.2 to 1.13

[oleibman]

When I correct the statement "$sheet = $excel->getActiveSheet()" to "$sheet = $spreadsheet->getActiveSheet()", the sheet saves correctly. Do you still see a problem if you make that change?

[lekoala]

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

[skillbox-koutja]

I have same trouble. Fix please

[lekoala]

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

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue for you, please try to help by debugging it further and sharing your results. Thank you for your contributions.

[lekoala]

This deserves some attention so I'll send a message again so that stalebot doesn't close this :)

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue for you, please try to help by debugging it further and sharing your results. Thank you for your contributions.

[Aerendir]

Did you find a solution to set the property?

[oleibman]

When I used a different formula =SOMEERORRORFUNCTION(, omitting the ending parenthesis, I do get an Exception. When I suppress the throw/trigger, the spreadsheet is generated. However, Excel complains when it tries to open it ("We found a problem with some content"); I am not convinced that this is a better result than not generating the spreadsheet in the first place, especially considering the information that the Exception message contains:

PhpOffice\PhpSpreadsheet\Calculation\Exception: Worksheet!A1 -> Formula Error: Expecting ')' ...

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.

[lekoala]

@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. I would expect that:

• It's configurable somewhere from the main spreadsheet object (or at the very least as a static property)
• That the cell containing the wrong formula has an empty result (and why not a comment with a detailed error message) so that you would still get a valid excel file at the end of the process. In some cases it's not worth it to prevent the entire file generation for one single error which can happen for dynamically generated documents

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.

[oleibman]

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 false, but any attempt by PhpSpreadsheet to alter the user's data (e.g. your suggestion of adding a comment) is way out of scope. Therefore Excel will still pop up "we found a problem" when the file is opened.

[lekoala]

@oleibman this is looking great

[oleibman]

Had to replace 3082 with 3091 for some reason. Essentially the same change.