Add more debug information for some specific archiving errors #16387
Conversation
| @@ -180,7 +180,7 @@ public function callAggregateAllPlugins($visits, $visitsConverted, $forceArchivi | |||
| $this->params->getSegment() ? sprintf("(for segment = '%s')", $this->params->getSegment()->getString()) : '' | |||
| ); | |||
| } catch (Exception $e) { | |||
| throw new PluginsArchiverException($e->getMessage() . " - in plugin $pluginName", $e->getCode(), $e); | |||
| throw new PluginsArchiverException($e->getMessage() . " - in plugin $pluginName with trace: " . $e->getTraceAsString(), $e->getCode(), $e); | |||
There was a problem hiding this comment.
Note: this should be handled when printing out an exception via ExceptionToTextProcessor::getWholeBacktrace
core/DataTable.php
Outdated
| @@ -1894,7 +1894,7 @@ protected function aggregateRowWithLabel(Row $row, $columnAggregationOps) | |||
| { | |||
| $labelToLookFor = $row->getColumn('label'); | |||
| if ($labelToLookFor === false) { | |||
| throw new Exception("Label column not found in the table to add in addDataTable()"); | |||
| throw new Exception("Label column not found in the table to add in addDataTable() for row " . var_export($row, 1)); | |||
There was a problem hiding this comment.
This happened in the GA importer, when debugging I found it would be more useful to have archiver params here, ie idSite/period/date and maybe report name. What do you think?
There was a problem hiding this comment.
That sounds good @diosmosis
btw is that bug issue fixed in the importer? I think the customer might have actually the issue with the importer
There was a problem hiding this comment.
If it's the same bug, then yes, it was an issue in the Referrers.getWebsites report, one of them will have a blank label causing period aggregation to fail this way. Can be fixed by re-importing the specific day(s).
Do you want to add the debug code or should I?
|
@diosmosis added some code for this. Is this what you meant? |
core/DataTable.php
Outdated
| $message = sprintf("Label column not found in the table to add in addDataTable(). idSite: %s, Period: %s, Row: %s", | ||
| DataTableFactory::getSiteIdFromMetadata($this), | ||
| DataTableFactory::getPeriodStringFromMetadata($this), | ||
| var_export($row, 1) |
There was a problem hiding this comment.
This data is added after querying, I don't think it will be available at this point in the Archiver. I haven't tested though, just saw this.
There was a problem hiding this comment.
I think it's needed to get the archive processor params being used, perhaps throwing a custom exception, then catching and decorating it would work. Just thinking off the top of my head.
|
@diosmosis removed that information again. I don't want to spend too much time on this as we don't have the problem anymore and we never really before had that issue. Getting data from archiveProcessor would require a lot of changes by the looks. |
|
👍 looks good, will merge if build passes EDIT: actually it could be useful to have eventually... for example, someone could archive a year report and see this because a single day has the problem and needs to be re-imported. Having the request string wouldn't say which day has the problem, but having the archive params might. I think I could add it pretty quickly (~15 mins at most) if it's still worth it. Let me know or feel free to merge. |
We are currently troubleshooting some archiving issue and this code we are customer to add in order to troubleshoot what is happening. This be great to have in general.