If I have text analysis enabled for a comment form and post a comment, I get WSOD although the comment does actually get posted.

The log says this:

Drupal\Core\Database\IntegrityConstraintViolationException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'reason' cannot be null:

I will try to investigate further in the next few days.

This is after the change I describe here https://www.drupal.org/node/2560147 to get the form to display.

CommentFileSizeAuthor
#11 2560153-11.patch1.03 KBmbaynton
#10 2560153-10.patch3.39 KBmbaynton
#4 2560153-4.patch418 bytestetranz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tetranz created an issue. See original summary.

tetranz’s picture

Title: Drupal\Core\Database\IntegrityConstraintViolationException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'reason' cannot be null: » SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'reason' cannot be null:
tetranz’s picture

I've had a look and this and the most obvious thing seems to be to set 'not null' => FALSE for the 'reason' column in mollom_schema().

I'm not sure what's changed. Perhaps Mollom was always returning a reason before but then it stopped. It doesn't return a reason for me on my free account when I post "ham".

tetranz’s picture

FileSize
418 bytes

I think this is all it needs.

tetranz’s picture

Assigned: Unassigned » tetranz
Status: Active » Needs review
eshta’s picture

Status: Needs review » Postponed (maintainer needs more info)

So we actually want the reason to be populated. I believe the root cause is actually an error somewhere before the write. In some of my recent work in this module I've seen this happen when there was an error returned from the Mollom API (like auth error). Can you check your logs to see if there is something like this?

tetranz’s picture

Thanks, I wondered if I had oversimplified it.

I will try to take a closer look over the weekend. I've learned a lot more about Drupal 8 since I was playing with this module a few weeks ago.

I just checked my site and the mollom table is empty even though I've been getting lots of comment spam as indicated in the watchdog log. I get a row in the table for ham but not spam.

I checked my old Drupal 7 site and that had a row for each spam submission although the reason column is all blank. It's a blank string, not a null.

tetranz’s picture

I've had another look at this and here are my findings.

It's kind of weird that you mentioned an auth error because on my dev machine, a laptop here at home, I am now receiving a 401 response from Mollom. I wasn't getting that when I looked at this a couple of weeks ago. I haven't been able to solve that. My credentials are validated successfully both when I save settings and on the first request when I post from a form. I don't know if Mollom is somehow blacklisting me or something.

My live site at Linode is still working fine. It's just a small hobby site that gets very few real comments but lots of spam so it's a good test. I captured some responses from there and brought them back to my dev machine for testing.

Here are typical responses for ham and spam. I changed code to codeX to avoid confusing the D.O. formatter.

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<contentresponse>
    <codeX>200</codeX>
    <content>
        <id>1509203f344aa3700b</id>
        <spamScore>0.0</spamScore>
        <spamClassification>ham</spamClassification>
    </content>
</contentresponse>
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<contentresponse>
    <codeX>200</codeX>
    <content>
        <id>1509201fb96e3c2be1</id>
        <spamScore>1.0</spamScore>
        <spamClassification>spam</spamClassification>
    </content>
</contentresponse>

There is no "reason". Do others receive a reason from Mollom? I only have a free account. Maybe a paid account provides it. The reason element remains null which is set in Drupal\mollom\Element::processMollom.

A related issue is that it only tries to save to the db with ham. It never attempts to save the analysis of spam which I assume is where "reason" would have some meaning. I think that is because, as expected, validation fails on spam but that means FormController::submitForm is never called and that's the only place ResponseDataStorage::save is called.

mbaynton’s picture

Just created a Mollom free account tonight and plugged my public/private keys into this module's HEAD; it resulted in a message that "Mollom servers confirmed my keys and everything is working" or something like that. However, I do get 401 Unauthorizeds back from Mollom on the spam check requests. So, that's a thing. But, the storage shouldn't puke this hard because of an unexpected Mollom response. In cases like this, is it desired to write a record to the database? If so, should we see about making 'Reason' be something that at least vaguely hints at an error occurred in the exchange with Mollom?

mbaynton’s picture

Status: Postponed (maintainer needs more info) » Needs review
FileSize
3.39 KB

Ok, I agree with @tetranz. When you don't get back a 401 (#2575951: OAuth signature computation bugs can help with that), there is indeed no "reason" given, for ham, anyway. According to the module's .install file, the reason column is supposed to store "A special reason for why a post was blocked", so we probably have correctly guessed that it is meant to store information from the mollom response, but more importantly by that description it would be expected to be valueless at least for posts that weren't blocked. And this is consistent with the responses we're seeing.

The schema gives default values for the things it asks be not null, as does the $defaults array in ResponseDataStorage::save. Reason is getting passed down to the database as null anyway because it is explicitly set null in the $data parameter passed in, but I think the schema default should be used instead in these cases. So, here's a patch that effectively overrides null values passed for non-nullable fields with the schema-specified default value for that field.

mbaynton’s picture

FileSize
1.03 KB

My bad, #10's patch had several issues. Here's the patch for just this issue.

eshta’s picture

I take this back - I think you are absolutely right that there are times when the reason field should be empty. However, the schema definition allows for this because there is a default value of ''. I think what is happening is that the NULL value is being explicitly set before saving rather than just being provided empty. I'm working on a fix right now. Hoping to have an updated D8 release out in the next day or so.

eshta’s picture

Status: Needs review » Needs work

  • eshta committed c53870d on 8.x-1.x
    Issue #2560153 by mbaynton: Updated OAUTH computation issues per Mollom...
  • eshta committed d3d34cb on 8.x-1.x
    Issue #2560153 by eshta, tetranz, mbaynton: Update null handling for...
eshta’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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

lilbebel’s picture

Hello,

I have Mollom 8.x-1.1 running on Drupal 8.1.1

I have enabled Mollom on the production site and now I am getting a WSOD when I try to create and save a node of a content type. My error log says:

Message Drupal\Core\Database\IntegrityConstraintViolationException: SQLSTATE[23000]: Integrity constraint violation: 19 NOT NULL constraint failed: node_field_data.title: INSERT INTO {node_field_data} (nid, vid, type, langcode, title, uid, status, created, changed, promote, sticky, revision_translation_affected, default_langcode, content_translation_source, content_translation_outdated) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?); Array ( [0] => 102 [1] => 102 [2] => practitioners [3] => en [4] => [5] => 1 [6] => 1 [7] => 1464655691 [8] => 1464655710 [9] => 0 [10] => 0 [11] => 1 [12] => 1 [13] => und [14] => 0 ) in Drupal\Core\Database\Connection->handleQueryException() (line 668 of /home/adaptiv9/public_html/core/lib/Drupal/Core/Database/Connection.php).

Thank you in advance for any help.

M

tetranz’s picture

Assigned: tetranz » Unassigned

I haven't touched Mollom for a long time. Unassigning myself.