aces
instrumentqueryengine doesn't handle null response to getFullName
Describe the bug
Our Loris instance hangs on loading the dataquery module. I tracked this down to an error in the dictionary/candidates endpoint, which returns HTTP 500:
Looking in the server error logs, this showed a stack trace from the instrumentqueryengine, which passes the results of $inst->getFullName() directly to the Category object constructor. When getFullName returns null, as is possible based on the typing ?string, it causes this crash. It's possible there is some unexpected issue with one of our instruments (see below), but the backend code should handle the null case gracefully regardless.
[Mon Mar 24 16:08:36.442317 2025] [php:error] [pid 740] [client <redacted>:50567] PHP Fatal error:
Uncaught TypeError: LORIS\\Data\\Dictionary\\Category::__construct():
Argument #2 ($desc) must be of type string, null given,
called in /var/www/loris/modules/instruments/php/instrumentqueryengine.class.inc on line 52 and
defined in /var/www/loris/src/Data/Dictionary/Category.php:26
Stack trace:
#0 /var/www/loris/modules/instruments/php/instrumentqueryengine.class.inc(52): LORIS\\Data\\Dictionary\\Category->__construct()
#1 /var/www/loris/modules/dictionary/php/module.class.inc(98): LORIS\\instruments\\InstrumentQueryEngine->getDataDictionary()
#2 /var/www/loris/modules/dictionary/php/categories.class.inc(34): LORIS\\dictionary\\Module->getUserModuleCategories()
#3 /var/www/loris/src/Middleware/UserPageDecorationMiddleware.php(247): LORIS\\dictionary\\Categories->handle()
#4 /var/www/loris/src/Middleware/PageDecorationMiddleware.php(58): LORIS\\Middleware\\UserPageDecorationMiddleware->process()
#5 /var/www/loris/php/libraries/NDB_Page.class.inc(725): LORIS\\Middleware\\PageDecorationMiddleware->process()
#6 /var/www/loris/php/libraries/Module.class.inc(321): NDB_Page->process()
#7 /var/www/loris/src/Middleware/ResponseGenerator.php(50): Module->handle()
#8 /var/www/loris/src/Middleware/AuthMiddleware.php(63): LORIS\\Middleware\\ResponseGenerator->process()
#9 /var/www/loris/src/Router/ModuleRouter.php(74): LORIS\\Middleware\\AuthMiddleware->process()
#10 /var/www/loris/src/Middleware/ExceptionHandlingMiddleware.php(54): LORIS\\Router\\ModuleRouter->handle()
#11 /var/www/loris/src/Router/BaseRouter.php(132): LORIS\\Middleware\\ExceptionHandlingMiddleware->process()
#12 /var/www/loris/src/Middleware/ResponseGenerator.php(50): LORIS\\Router\\BaseRouter->handle()
#13 /var/www/loris/src/Middleware/ContentLength.php(52): LORIS\\Middleware\\ResponseGenerator->process()
#14 /var/www/loris/htdocs/index.php(73): LORIS\\Middleware\\ContentLength->process()
#15 {main}\n thrown in /var/www/loris/src/Data/Dictionary/Category.php on line 26
On further inspection, it appears that the issue is related to #9526 . I was able to reproduce the issue with a single instrument added to the Loris instance, and none of our other instruments are causing this issue. The instrument at fault is named study_status, and has initial lines:
table{@}study_status
title{@}study_status
It seems likely these are being skipped in the linst loader due to names with _status included appearing as the second field of the lines, although they are not field names of the instrument. If Loris is unable to host instruments ending in _status for some reason, would appreciate a check, but if the restriction only applies to field names, then at minimum it would be great to avoid this check for table and title lines.
To Reproduce
- Add a linst instrument with table and title name study_status.
- Navigate to dataquery beta module
What did you expect to happen?
If it is not possible to use the dataquery module without the full instrument name, then I would expect an error message to be displayed, with information on the instrument that is causing the issue. If possible, however, I would expect the dataquery module to load without this.
Browser Environment:
- OS: MacOS
- Browser: Chrome
- Version: 134.0.6998.118
Server Environment:
- LORIS Version: 26.0.0
- Linux distribution and Version: Ubuntu 22.04
- MySQL/MariaDB Version: MySQL 9.2 Community
Additional context
-- outdated, as I have found the issue that allows reproduction, but left as context in case helpful -- Our instruments are a bit odd in that they are converted from REDCap instruments by script, then added to Loris in the docker container initialization by running generate_tables_sql_and_testNames.php and inserting the resulting SQL file directly to the database (ref: https://github.com/childmindresearch/loris-docker/blob/main/loris/install-loris.sh#L102). If there's any obvious issues with this method, please let me know so I can fix the container build.
Thank you for the detailed report. Without having had time to look into it in depth a couple quick points:
- the _status field names are special in LORIS and skipped by the LINST parser for the reasons mentioned in the issue you linked. There's no reason for them to be forbidden from test names or titles. It's likely just that the parser is overly broad for catching _status before checking the type.
- I don't know why getFullName would be a nullable type, that doesn't make sense to me (though you're right that it does seem to be nullable in the code.) @ridz1208 do you have any ideas why?
