Problem/Motivation

When a non existing field key in Conditions is set and we try to access the sensor a fatal error occurs.
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42S22]: Column not found: 1054 Unknown column '%' in 'where clause'

Steps to reproduce:
Go to admin/config/system/monitoring/sensors/add.
Set a label and select Database Aggregator.
Set table as watchdog, any string in Conditions and 'wid' in Verbose fields.
Save.
Go to the link of the created sensor.

Proposed resolution

We should verified that the field key exist in the table.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#6 2557977-6.patch1.92 KBgiancarlosotelo
#6 2557977-TESTONLY.patch1.01 KBgiancarlosotelo
#2 2557977-2.patch936 bytesgiancarlosotelo

Comments

giancarlosotelo created an issue. See original summary.

giancarlosotelo’s picture

giancarlosotelo commented
Status: Active » Needs review
StatusFileSize
new 2557977-2.patch936 bytes

Adding a try/catch would solve this problem.

Status: Needs review » Needs work

The last submitted patch, 2: 2557977-2.patch, failed testing.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

The resultVerbose function should not throw an exception.
This situation needs to be caught internally inside the verbose processing. Also we can display a proper error message in such a case - not just verbose message markup.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

I'm actually OK with fixing it like this. We do it exactly the same for the actual sensor check too.

giancarlosotelo’s picture

giancarlosotelo commented
Status: Needs work » Needs review
StatusFileSize
new 2557977-TESTONLY.patch1.01 KB
new 2557977-6.patch1.92 KB

Well I added test for this error creating a Database Sensor with invalids conditions and asserting that the sensor has no problems otherwise should fail.

LKS90’s picture

LKS90 commented

Works correctly, I'm just not sure about the test. It'd be nicer if it explicitly failed instead of just producing one exception. (You could do that with another try/catch() block where you call the runSensor() method)

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Fixed

The test is fine. It just needs to fail then.

Committed.

giancarlosotelo’s picture

giancarlosotelo commented

Can this issue be related to #2528528: Test DatabaseAggregator sensor with invalid fields ?

We are testing DatabaseAggregator with wrong values. or it is more related to UI Test?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

The last submitted patch, 6: 2557977-TESTONLY.patch, failed testing.

Status: Closed (fixed) » Needs work

The last submitted patch, 6: 2557977-6.patch, failed testing.

juanse254’s picture

juanse254 commented
Status: Needs work » Closed (fixed)

The last submitted patch, 2: 2557977-2.patch, failed testing.

The last submitted patch, 6: 2557977-TESTONLY.patch, failed testing.

Status: Closed (fixed) » Needs work

The last submitted patch, 6: 2557977-6.patch, failed testing.

giancarlosotelo’s picture

giancarlosotelo commented
Status: Needs work » Closed (fixed)