Skip to content
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

Jump to bottom

Fatal error in Goals/Pages.php, report is null #13883

Closed
tsteur opened this issue Dec 19, 2018 · 3 comments · Fixed by #13892
Closed

Fatal error in Goals/Pages.php, report is null #13883

tsteur opened this issue Dec 19, 2018 · 3 comments · Fixed by #13892
Assignees
@tsteur
Labels
Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Milestone
3.8.0

Comments

@tsteur
Copy link
Member

tsteur commented Dec 19, 2018

Here's a stacktrace

Error: {"message":"Argument 1 passed to Piwik\Report\ReportWidgetFactory::__construct() must be an instance of Piwik\Plugin\Report, null given, called in /plugins/Goals/Pages.php on line 345","file":"/core/Report/ReportWidgetFactory.php","line":33,"backtrace":" on /core/Report/ReportWidgetFactory.php(33)\n#0 /plugins/Goals/Pages.php(345): Piwik\Report\ReportWidgetFactory->__construct(NULL)\n#1 /plugins/Goals/Pages.php(297): Piwik\Plugins\Goals\Pages->createWidgetForReport('CustomDimension...', 'getCustomDimens...')\n#2 /plugins/Goals/Pages.php(83): Piwik\Plugins\Goals\Pages->buildGoalByDimensionView('', Object(Piwik\Widget\WidgetContainerConfig))\n#3 /plugins/Goals/Reports/Get.php(75): Piwik\Plugins\Goals\Pages->createGoalsOverviewPage(Array)\n#4 /core/Widget/WidgetsList.php(199): Piwik\Plugins\Goals\Reports\Get->configureWidgets(Object(Piwik\Widget\WidgetsList), Object(Piwik\Report\ReportWidgetFactory))\n#5 /plugins/API/API.php(370): Piwik\Widget\WidgetsList::get()\n#6 [internal function]: Piwik\Plugins\API\API->getWidgetMetadata('2')\n#7 /core/API/Proxy.php(232): call_user_func_array(Array, Array)\n#8 /core/Context.php(28): Piwik\API\Proxy->Piwik\API\{closure}()\n#9 /core/API/Proxy.php(323): Piwik\Context::executeWithQueryParameters(Array, Object(Closure))\n#10 /core/API/Request.php(263): Piwik\API\Proxy->call('\\Piwik\\Plugins\\...', 'getWidgetMetada...', Array)\n#11 /plugins/API/Controller.php(41): Piwik\API\Request->process()\n#12 [internal function]: Piwik\Plugins\API\Controller->index()\n#13 /core/FrontController.php(556): call_user_func_array(Array, Array)\n#14 /core/FrontController.php(144): Piwik\FrontController->doDispatch('API', false, Array)\n#15 /core/dispatch.php(34): Piwik\FrontController->dispatch()\n#16 /index.php(27): require_once('...')\n#17 {main}"}

URL: ?module=API&method=API.getWidgetMetadata&filter_limit=-1&format=JSON&deep=1&idSite=2

Referrer:?module=CoreHome&action=index&date=yesterday&period=day&idSite=2

Happens here: https://github.com/matomo-org/matomo/blob/3.8.0-b5/plugins/Goals/Pages.php#L344-L345

I was going to create a PR that checks whether $report was found and if not, return null and just skip this report/widget but not sure if this would maybe just suppress the error while there is a problem with an actual implementation. Cause in theory it iterates over all found reports and it should find a report class. For custom dimensions (which is the problem see stack trace) there were a few changes around the reports and widgets recently so maybe something regressed there?

I can check for the dashboard configuration if needed. Moving it into 3.8 as the dashboard cannot be loaded in this case by the looks
@tsteur tsteur added the Regression Indicates a feature used to work in a certain way but it no longer does even though it should. label Dec 19, 2018
@tsteur tsteur added this to the 3.8.0 milestone Dec 19, 2018
@tsteur
Copy link
Member Author

tsteur commented Dec 19, 2018

Just checked there seems to be no dashboard configured.

One custom dimension is defined

INSERT INTO `custom_dimensions` (`idcustomdimension`, `idsite`, `name`, `index`, `scope`, `active`, `extractions`, `case_sensitive`)
VALUES
	(1, 2, 'ID Foo', 1, 'visit', 1, '[]', 1);

@tsteur
Copy link
Member Author

tsteur commented Dec 19, 2018 •
edited

Closing the issue for now. I've tried to debug locally and reproduce the issue locally as well as on the actual instance but couldn't reproduce it. If it occurs again need to check. Seems some race condition maybe or so.

@tsteur tsteur closed this as completed Dec 19, 2018
@tsteur tsteur added wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. worksforme The issue cannot be reproduced and things work as intended. and removed Regression Indicates a feature used to work in a certain way but it no longer does even though it should. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. labels Dec 19, 2018
@tsteur tsteur reopened this Dec 20, 2018
@tsteur
Copy link
Member Author

tsteur commented Dec 20, 2018 •
edited

Same issue happened again. Seems to happen very rarely. Possibly a race condition and maybe it is actually easiest to just check whether report was returned and if not, don't add the widget as it seems to happen very very rarely.

@tsteur tsteur added Regression Indicates a feature used to work in a certain way but it no longer does even though it should. and removed worksforme The issue cannot be reproduced and things work as intended. labels Dec 20, 2018
This was referenced Dec 20, 2018
Merged
Closed
@tsteur tsteur self-assigned this Dec 20, 2018
tsteur added a commit that referenced this issue Dec 21, 2018
@tsteur
Better check whether report is cached
0102f4c 
refs #13883 and #13892

Checking for `$cache->contains($lazyCacheId)` and then `$cache->fetch($lazyCacheId)` can run into race conditions when the cache expires just in between. I reckon this might be the case for #13892 which we see happening like once a day or every other day.

We would possibly also need to check for other usages. Might not actually be an issue cause the transient is used in the lazy cache as well and it should keep the entry during the same request. It could come to this issue though the first time a cache entry is read.
@tsteur tsteur mentioned this issue Dec 21, 2018
Merged
diosmosis pushed a commit that referenced this issue Jan 22, 2019
@tsteur @diosmosis
Better check whether report is cached (#13902)
0ca1569 
refs #13883 and #13892

Checking for `$cache->contains($lazyCacheId)` and then `$cache->fetch($lazyCacheId)` can run into race conditions when the cache expires just in between. I reckon this might be the case for #13892 which we see happening like once a day or every other day.

We would possibly also need to check for other usages. Might not actually be an issue cause the transient is used in the lazy cache as well and it should keep the entry during the same request. It could come to this issue though the first time a cache entry is read.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsteur tsteur

Labels
Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Milestone
3.8.0
Development

Successfully merging a pull request may close this issue.

Prevent possible fatal error when report not found matomo-org/matomo
1 participant
@tsteur