WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 6 months ago

Last modified 4 months ago

#46898 closed defect (bug) (fixed)

WSOD Protection: Finalize email language

Reported by: TimothyBlynJacobs Owned by: SergeyBiryukov
Milestone: 5.2 Priority: normal
Severity: normal Version: 5.2
Component: Site Health Keywords: servehappy
Focuses: Cc:
PR Number:

Description

The email content received when the site experiences an error needs to be finalized.

https://github.com/WordPress/servehappy/issues/40 for reference

Attachments (7)

46898.diff (1.2 KB) - added by TimothyBlynJacobs 6 months ago.
46898.2.diff (1.2 KB) - added by TimothyBlynJacobs 6 months ago.
46898.3.diff (1.1 KB) - added by TimothyBlynJacobs 6 months ago.
46898.4.diff (1.7 KB) - added by TimothyBlynJacobs 6 months ago.
46898.5.diff (1.6 KB) - added by desrosj 6 months ago.
Changes string per last comment
46898.6.diff (4.0 KB) - added by pento 6 months ago.
46898.6.alt.diff (6.6 KB) - added by SergeyBiryukov 6 months ago.

Download all attachments as: .zip

Change History (34)

#1 @desrosj
6 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 5.2

#2 @ocean90
6 months ago

  • Keywords needs-refresh added

These are the current signatures we use in new site and update emails:

Thanks! -- The WordPress Team
The WordPress Team
--The WordPress Team
https://wordpress.org/

We should use one of them for this email too.

#3 @TimothyBlynJacobs
6 months ago

  • Keywords needs-refresh removed

Updated to use the last option because it was closest to what @miss_jwo proposed in GitHub, but I don't have a strong preference either way.


#4 @birgire
6 months ago

I was just wondering about the TBD in the email body :-) I see it's being taken care of in 46898.3.diff.

There are some plugins that end with "Plugin" in their name:

https://wordpress.org/plugins/search/plugin

So one consideration regarding the strings:

'This was caused by the %s plugin.'
'This was caused by the %s theme.'

used in the email body, is to use instead:

'This was caused by the plugin: %s.'
'This was caused by the theme: %s.'

to avoid adjacent cases like:

This was caused by the Some Plugin plugin.

and have instead

This was caused by the plugin: Some Plugin.

Or use some another approach around this.

#5 @TimothyBlynJacobs
6 months ago

That sounds like a good adjustment to me @birgire! Added in .4

This ticket was mentioned in Slack in #core by desrosj. View the logs.


6 months ago

#7 @desrosj
6 months ago

This was caused by the plugin: Some Plugin. feels a bit awkward to me.

How about This was caused by the following plugin: Some Plugin.?

@desrosj
6 months ago

Changes string per last comment

#8 @TimothyBlynJacobs
6 months ago

That sounds good to me!

#9 @SergeyBiryukov
6 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 45181:

Bootstrap/Load: Finalize recovery mode email language.

Props TimothyBlynJacobs, miss_jwo, desrosj.
Fixes #46898.

#10 @pento
6 months ago

  • Keywords has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening, as the language in this email is scary and confusing. I'm aware that we're past the soft string freeze, but I don't think we can reasonably release it as is.

@matt noted:

I really thought my site had been hacked when I got that email
and didn't click on the link since I wasn't sure what it would do

If a highly technical site owner is confused by the email, I'm quite concerned about what the 99.999% of site owners will do when they see the same.

For reference, here's a copy of the current email text:

Howdy,

Your site recently crashed and may not be working as expected.

This was caused by the following plugin: test.php.

Please click the link below to initiate recovery mode and fix the problem.

This link expires in 4 hours.

http://localhost:9999/wp-login.php?action=enter_recovery_mode&rm_key=...

Error Details
=============
An error of type E_WARNING was caused in line 6 of the file /var/www/src/wp-content/plugins/test.php. Error message: count(): Parameter must be an array or an object that implements Countable

--The WordPress Team
https://wordpress.org/

There are a few points on the content that come to mind:

  • Opening with "Your site recently crashed" is too strong. It feels panic-inducing.
  • The single sentence paragraphs are too short, they don't offer any additional information about why this email has arrived.
  • "This link expires in 4 hours" inspires artificial urgency, and it's not similar to the language of email scams.
  • The "Error Details" section is entirely unnecessary for the vast majority of site owners. Maybe only include this if WP_DEBUG_DISPLAY is true. It'd be more useful to report this information (in an anonymised fashion) to w.org, so it could be aggregated and plugin/theme authors could be made aware of it.

Additionally, sending the email every 4 hours feels overly aggressive. Could someone point me to the original discussion on the link timeout, and why 4 hours was chosen?

Also, I saw this email being triggered by a cron task, which seems overly obtuse. At most the site owner would want to report that error to the plugin author, not take immediate recovery action.

Here's a first pass at some new text:

Howdy!

When a plugin or theme on your site runs into technical issues, WordPress will automatically catch that error, and send you an email to let you know what happened.

In this case, WordPress caught a potential issue with one of your plugins, Hello Dolly.

You should first check if your website (https://my-amazing-site.boop/) is running normally. In particular, the error that WordPress caught happened on https://my-amazing-site.boop/cart/checkout. Try visiting that page and see if it looks correct to you.

If everything appears to be working, the author of Hello Dolly would greatly appreciate if you could post more information about this error in the support forums. You can find the error details at the end of this email.

https://wordpress.org/support/plugin/hello-dolly/

If there's something not quite right about your site, and you can't access your dashboard normally, WordPress has a special "recovery mode", which allows you to safely login to your dashboard and disable the Hello Dolly plugin.

https://my-amazing-site.boop/wp-login.php?action=enter_recovery_mode&rm_key=...

To keep your site safe, this link will expire in 4 hours. Don't worry about that, though: a new link will be emailed to you if you click on it after it expires.

--The WordPress Team
https://wordpress.org/

Error Details
=============
An error of type E_WARNING was caused in line 14 of the file /var/www/src/wp-content/plugins/hello.php. Error message: count(): Parameter must be an array or an object that implements Countable

I've asked some editorial folks to also review this issue.

#11 @karmatosed
6 months ago

+1 for making the text less scary. I know some edits were made but looking at this we can go a lot further and should. Let's take this opportunity to sooth over scare.

#12 @michelleweber
6 months ago

Yes for less scary! A couple of questions about a few lines:

technical issues
potential issues

I am all for less scary, but too vague is also a little scary because then I have no idea what might be wrong with my site or how bad the problem is. Is there any additional detail we can offer here to contextualize?

If everything appears to be working, the author of Hello Dolly would greatly appreciate if you could post more information about this error in the support forums. You can find the error details at the end of this email.

Assume I am not a technical person -- if everything appears to be working, then what error am I posting about? Why are there error details if my site is fine? I think we want to be super direct that we want them to report that they don't see any problems but WordPress identified X error, and that they should copypaste what they see at the bottom of the email.

If there's something not quite right about your site, and you can't access your dashboard normally

"And you can't access" or "or you can't access"? And on "not quite right," same issue re: vague also being scary. "Not quite right" seems to minimize things, but then not being able to get to my dashboard seems like a pretty big deal. Can we give more detail, or offer examples of what might seem off? Or can/should we just say "If anything at all seems abnormal to you or you can't access your dashboard normally"?

#13 follow-up: @matt
6 months ago

It should explain itself, ie: Since WordPress X.X there has been a built-in feature that detects when a plugin causes a fatal error on your site, and instead of breaking your site, it deactivates the plugin and sends you this automated email.

It's not from the WP team, it's from your site. If they want to get in touch to follow up .org is not the right place, it's really their web host. Perhaps we could add a filter there web hosts could hook into, if a variable is defined: Your web host, XYZ, has suggested if you need more help you get in touch with them via STRING.

#14 @TimothyBlynJacobs
6 months ago

I'm +1 to any language changes, just wanted to provide technical clarity.

Additionally, sending the email every 4 hours feels overly aggressive. Could someone point me to the original discussion on the link timeout, and why 4 hours was chosen?

Also, I saw this email being triggered by a cron task, which seems overly obtuse. At most the site owner would want to report that error to the plugin author, not take immediate recovery action.

The email isn't triggered by a cron task. It's sent whenever the fatal error happens immediately and if the error continues to happen, it will send a new recovery mode link at most every four hours.

The original timeout was one hour, but was revised in this slack discussion: https://wordpress.slack.com/archives/C60K3MP2Q/p1550761315015300 and discussed briefly in other meetings.

The trade off being you don't want to bombard the user with emails, but you don't want to make the timeout between emails so long that getting to the recovery mode link would be very difficult.

"This link expires in 4 hours" inspires artificial urgency

Right now, the link itself is valid only for four hours. We could dramatically increase this, perhaps to 7 days, such that it isn't necessary to mention.

The "Error Details" section is entirely unnecessary for the vast majority of site owners.

This was added because we don't store the error server side until the user engages in recovery mode. So it provides parity with the stack trace that is shown in wp-admin. That being said, I'm fine with completely removing it, or including it on WP_DEBUG_DISPLAY like you mentioned. Perhaps also a way to filter on?

This ticket was mentioned in Slack in #core-php by schlessera. View the logs.


6 months ago

#16 @schlessera
6 months ago

During the core-php meeting we discussed how to handle resending an email for a given error, as we're not storing the caught error. The reason why we'd need that is that the recovery email generation needs it for two pieces of information: a.) the plugin/theme name that caused the issue and b.) the error details at the end.

If we choose to not display the error details at the end, we should also consider not putting the plugin/theme name into the email itself, but provide more generic language, knowing that the admin backend will contain all the details one needs to diagnose further.

Something along the lines of:
There was an issue with one or more of your plugins or themes. Log into the admin backend through the provided recovery link to see more details.

This way, we don't need to pass an error object to the email generation, and can thus easily regenerate emails for expired tokens.

#17 in reply to: ↑ 13 @johnbillion
6 months ago

Replying to matt:

It's not from the WP team, it's from your site.

Related: #46057.

#18 @spacedmonkey
6 months ago

Related: #46950

This is when the email is sent and the wording to users on screen.

This ticket was mentioned in Slack in #core by chanthaboune. View the logs.


6 months ago

#20 @chanthaboune
6 months ago

Forgive my formatting, I'm not a Trac native.

Most of the feedback on the text was around making it more self-explanatory, a little closer to everyday language, and a better balance between vague/not-too-vague. I'd love a second opinion on it.

What I updated:

  • Pulled in Matt's opening paragraph.
  • Made the step-wise directions more clearly step-wise in full para two
  • Mirrored "error", "broken", "issues" etc between related sentences for continuity and clarity.
  • Balanced the length & complexity of most sentences, and erred on the "everyday language" side of the explanations.
Howdy!

Since WordPress 5.2 there is a built-in feature that detects when a plugin causes a fatal error on your site and, instead of breaking your site, it deactivates the plugin and sends you this automated email. 

In this case, WordPress caught an error with one of your plugins, Hello Dolly.

First, visit your website (https://my-amazing-site.boop/) and check for any visible issues. Next, visit the page where the error was caught (https://my-amazing-site.boop/cart/checkout) and check for any visible issues.

If nothing looks broken, the author of Hello Dolly would greatly appreciate if you could post more information about this error in the support forums. You can find the error details at the end of this email.

https://wordpress.org/support/plugin/hello-dolly/

If your site appears broken and you can't access your dashboard normally, WordPress now has a special "recovery mode". This lets you safely login to your dashboard and disable the Hello Dolly plugin.

https://my-amazing-site.boop/wp-login.php?action=enter_recovery_mode&rm_key=...

To keep your site safe, this link will expire in 4 hours. Don't worry about that, though: a new link will be emailed to you if you click on it after it expires.

Error Details
=============
An error of type E_WARNING was caused in line 14 of the file /var/www/src/wp-content/plugins/hello.php. Error message: count(): Parameter must be an array or an object that implements Countable

@pento
6 months ago

#21 follow-up: @pento
6 months ago

In 46898.6.diff:

  • Increase the link expiry to 1 day.
  • Ensure that plugins which aren't in a directory (eg, hello.php) are detected correctly.
  • Added a translator comment for the email text.
  • Update the email text. Based on @chanthaboune's text above, but with some bonus changes:
    • Refer to "plugin or theme" instead of just "plugin".
    • Change the first sentence, as the plugin won't be deactivated automatically. The site owner is just notified.
    • Replaced the suggestion to visit the plugin forums, with a suggestion to contact your host. This message is filterable with the new recovery_email_support_info filter.
    • Removed the reference to the plugin name from the recovery mode link explanation, as this was a little too tricky to insert in a nicely translatable way.
Howdy!

Since WordPress 5.2 there is a built-in feature that detects when a plugin or theme causes a fatal error on your site, and notifies you with this automated email.

In this case, WordPress caught an error with one of your plugins, Hello Dolly.

First, visit your website (http://localhost:9999/) and check for any visible issues. Next, visit the page where the error was caught (http://localhost:9999) and check for any visible issues.

Please contact your host for assistance with investigating this issue further.

If your site appears broken and you can't access your dashboard normally, WordPress now has a special "recovery mode". This lets you safely login to your dashboard and investigate further.

http://localhost:9999/wp-login.php?action=enter_recovery_mode&rm_token=...

To keep your site safe, this link will expire in 1 day. Don't worry about that, though: a new link will be emailed to you if the error occurs again after it expires.



Error Details
=============
An error of type E_WARNING was caused in line 15 of the file /var/www/src/wp-content/plugins/hello.php. Error message: Division by zero
Last edited 6 months ago by pento (previous) (diff)

#22 @pento
6 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 45268:

Bootstrap/Load: Tweak the recovery mode email text and behaviour.

  • Change the recovery mode link expiry to 1 day.
  • Improve the email text.
  • Add a new recovery_email_support_info filter, for hosts to be able to customise their support contact information.

Props pento, chanthaboune, michelleweber, matt.
Fixes #46898.

#23 in reply to: ↑ 21 @SergeyBiryukov
6 months ago

Replying to pento:

Removed the reference to the plugin name from the recovery mode link explanation, as this was a little too tricky to insert in a nicely translatable way.

Was playing with this too, 46898.6.alt.diff does that. But the support forums link isn't always accurate, and I like the filter in [45268] more :)

#24 @pento
6 months ago

Nice! I poked in that direction a little, but ran into two main problems:

#25 @WFMattR
6 months ago

Where the email body says "Next, visit the page where the error was caught (http:// ...)", is there any concern that inexperienced users may click URLs that could be dangerous?

46898.6.alt.diff uses home_url( $_SERVER['REQUEST_URI'] ), which would include any query string parameters. An attacker could potentially trigger a fatal error with a URL that is also dangerous for a logged-in admin to visit. Although there are significant limitations (including triggering a fatal error and using a malicious query string parameter in a single request, that's processed before the fatal error), in some plugins/themes, this could let an attacker send a malicious link to an admin inside a legitimate email message, without having to know their address.

#26 @TimothyBlynJacobs
6 months ago

is there any concern that inexperienced users may click URLs that could be dangerous?

This was discussed in passing in #core-php as well, https://wordpress.slack.com/archives/C60K3MP2Q/p1555347605059600

#27 @spacedmonkey
4 months ago

  • Component changed from Administration to Site Health
Note: See TracTickets for help on using tickets.