TypeError if InsertTag is combined with ID of non-existent Node-Element

[tarun-uwe]

If an ID of a non-existent Node-Element is refrenced in an InsertTag, an Server error 500 is produced.

Error message Return value of Terminal42\NodeBundle\EventListener\InsertTagsListener::generateNodes() must be of the type string, null returned

Versions Contao: 4.7.7 and 4.9.5 Node: Current Release 1.3.1

Reason The InsertTagListener function generateNodes is declared to return a string. It calls generateSinglein Line 68 https://github.com/terminal42/contao-node/blob/81301a92f69a7a5463854feb766ccec5e5b0d523/src/EventListener/InsertTagsListener.php#L60-L68

The NodeManager function generateSingle is declared as nullable | string. If null is returned, the TypeError occurs. https://github.com/terminal42/contao-node/blob/81301a92f69a7a5463854feb766ccec5e5b0d523/src/NodeManager.php#L28-L38

Solution? Check in generateNodesif the returned value is a string and else return an empty string. or Return an empty string from generateSingleinstead of null.

Screenshot

[qzminski]

In my opinion the current approach is perfectly fine. @Toflar ?

[fritzmg]

imho generateNodes should at least check if the return value of generateSingle is null and then throw a more appropriate exception.

Though more commonly, misconfigured insert tags are usually ignored or only log an error to the system log, instead of showing an error in the front end.

[tarun-uwe]

In my opinion the current approach is perfectly fine. @Toflar ?

I think the current approach is not consistent.

• If there is no id --> the EventListener returns an empty String.
• If there is one "non-existing" id --> it fails and throws an error. (insert_node Tag)
• If there are multiple "non-existing" ids --> it returns an empty String. (insert_nodes Tag) return implode("\n", $this->manager->generateMultiple($ids));

Though more commonly, misconfigured insert tags are usually ignored or only log an error to the system log, instead of showing an error in the front end.

From a user-perspective, I think it would be preferable, to ignore it or write a log entry / log an error. If someone by mistake deletes a Node Element, which is used in a Footer or Header of a website, the complete website would not be reachable.

[Toflar]

I think not returning anything and adding a log entry is kind of the typical way how Contao solves this. Will be a bit tricky for the generateMultiple but could be solved by a 2nd argument array &$idsNotFound which we could fill in case their missing so the listener could generate log entries for those as well.

I consider this an enhancement though because functionality isn't breaking when used properly.

Feel free to work on a PR.

[aschempp]

Implemented in 89828f6b5a0e843dc1d4655e7e4c07cbfe3088ca