Problem/Motivation

Before #2987444: Ajax errors are not communicated through the UI there was no way to know via the UI that an Ajax error occurred. The only evidence of it happening was in the JS console.

In #2987444: Ajax errors are not communicated through the UI, functionality was added so if there is an Ajax error, this is made evident in the UI with the following message generated by the messages API.

Oops, something went wrong. Check your browser's developer console for more details.

While the specifics of the error still require opening the JS console, the fact that an error took place is actually apparent, which is a big improvement.
Because it's difficult to find the ideal wording for pretty much anything this issue exists to discuss potential improvements to the content of that message. If a largely-agreed-upon better option emerges here, we can update core to use that phrasing instead. There were clear benefits to getting that functionality into core as soon as possible with acceptable messaging, and deferring the composition of ideal messaging to a followup.

Proposed resolution

Agree on a better default message.

Remaining tasks

Agree on, and then set, a better default message.

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3349901

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

bnjmnm created an issue. See original summary.

quietone’s picture

@bnjmnm, thanks for making this issue.

I am concerned that the message does not help a non technical user. Something like the existing message, "Contact the site administrator if the problem persists." would point them in the right direction.

lauriii’s picture

Title: [PP-1] Determine if there is a better message for AJAX errors. » Determine if there is a better message for AJAX errors.
Status: Postponed » Active

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

askibinski’s picture

What about making it configurable under admin/config/development/logging?

aporie’s picture

StatusFileSize
new890 bytes

I agree.

The message is not clear. Plus, we should think about displaying a message only on error 500.

I guess this feature was made thinking about using Ajax with drupal css "use-ajax" and "use-ajax-submit" but you want to let developers be able to handle other errors themselves (such as 403 or whatever) on Drupal.ajax implementations.

I personally ended up creating this patch for 10.1

glugmeister’s picture

StatusFileSize
new716 bytes

We have a multi-step webform with ajax enabled. In user testing, a user got the "Oops" error when their device temporarily lost internet connection. With the patch in #6, the user sees no error and has no clue something is wrong. The webform simply doesn't respond. Here is another patch which which shows two different error messages depending on the XMLHttpRequest status:

  1. Status 0 (request did not complete): "Oops, something went wrong. Check your internet connection and try again."
  2. All other status codes: "Oops, something went wrong. Contact the administrator."
tаo’s picture

StatusFileSize
new155.19 KB

I see two problems with this functionality.
1. There is no conventional way to change or hide this message from display if necessary. (why can't it be just configurable?)
2. Each error displays every time on the new line and it looks wrong when some AJAX query repeats from time to time (Notifications as an example)

aporie’s picture

Regarding #7 and #8, from my POV, error handling should be done by project, and it's almost impossible to provide an all-in-one solution which will satisfy each Drupal website.

Hence, the idea of only handling error 500 or specific errors like maybe status code 0, even though this can be produced because of different reasons, see here and here.

Even making a big config form with a custom message for each HTTP status code wouldn't make sense to me, as, for example, you might want to return a different error message on different 403 or 404 to your users on the same AJAX response:

Example, errors 403 returned by the same Ajax handler:

  • You do not have the necessary permissions to access this resource.
  • Your session has expired.
  • Insufficient privileges to perform the requested action.
  • Access to this resource is restricted from your current IP address.
  • etc. etc.
james.williams’s picture

In the spirit of continuing to improve the messaging, even if further follow-ups could be done beyond that, how about we just make the existing message configurable? The tone of the current message is simply inappropriate for many sites, let alone whether it could be more helpful, or different for various scenarios.

james.williams’s picture

Issue summary: View changes
Status: Active » Needs work
Issue tags: +Needs tests

I've made a start on at least making the contents of the message configurable in https://git.drupalcode.org/project/drupal/-/merge_requests/8880 in the hope that this could help. I'm well aware some new tests are needed, but hopefully this start demonstrates my thinking. I've tried to take into account what's already been said on this ticket, and on the original #2987444: Ajax errors are not communicated through the UI.

If we made the contents of the message configurable like this, perhaps actually determining a better 'default' message could be a lower priority follow-up? At least this way sites can own the tone of their error message.

I'm also aware that maybe a follow-up could be needed to determine if there is a better message to describe AJAX errors on the configuration form, hah! At least we may be able to assume a bit more technical capability in that context. There's already mention of 'AJAX' in views' administration forms, for example, so I think it would be OK to mention AJAX there, even if the message itself should not.

james.williams changed the visibility of the branch 3349901-d10-2-configurable-ajax-error-msg to hidden.

james.williams’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs tests

Added test that a changed error message does indeed show on an AJAX exception, which passes. (The test rightly fails in the tests-only scenario, because trying to save the new config property gets flagged as unsupported key.)

james.williams’s picture

Title: Determine if there is a better message for AJAX errors. » Allow changing the message for AJAX errors
Issue summary: View changes

Adjusted issue summary etc as allowing the message to be customised might be a more achievable goal than actually finding & agreeing a better default message. I won't be offended if others disagree and this should be reverted! Just looking to keep this moving in a useful direction.

longwave’s picture

The string is wrapped in Drupal.t() so you should be able to use the standard translation mechanisms to alter the message already? We don't usually provide explicit settings for user-facing strings for this reason - the translation system is preferred, and it also handles the case of multiple languages by default (which I'm not sure the proposed solution does?)

We could still consider improving the default message in this issue.

james.williams’s picture

Title: Allow changing the message for AJAX errors » Determine if there is a better message for AJAX errors
Issue summary: View changes
Status: Needs review » Needs work

@longwave thanks for your thoughts. So on a technical level, I can accept that translation could be the only option. But more pragmatically, if I have a enterprise client whose site is only in English, so need none of the language/translation modules, their only option would be to use hardcoded custom strings ($settings['locale_custom_strings_en']), or some extra module to allow translating English interface strings? That's a bit disappointing, but I'll accept it if that's definitely the thinking.

For what it's worth, I did set the new config property to be translatable (via the label type), and handle copying any existing interface translations for the existing string into language config overrides in the update hook in my MR.

Reverting my changes to the issue title & description now.

longwave’s picture

Status: Needs work » Needs review

FWIW for sites where developers are purely in control of the strings I use the $settings option, or if the customer wants to modify strings themselves you can still install locale and interface translation modules, but only have English language installed, and let them edit the English strings in the UI.

james.williams’s picture

Makes sense. The issue I have with that is that the locale module depends on the language module, which then 'pollutes' every entity display with its widgets/components, even when there's only one available language. Which might be fine in many cases, but is much less desirable for my client's case (which already involves a fair bit of complexity in its configuration, with splits for various brands etc).

Anyway, back to the attempt to refine the actual message. How about removing the 'Oops', and incorporating the suggestion from @quietone back in comment 1:

Something went wrong. Your browser's developer console contains more details. Contact the site administrator if the problem persists.

The 'oops' reminds me of the message that was removed from the redirect module over a decade ago - and I reckon Drupal's audience has probably shifted further away from where that tone is normal in the meantime.

james.williams changed the visibility of the branch 3349901-configurable-ajax-error-msg to hidden.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

james.williams’s picture

Status: Needs work » Needs review
hannahdigidev’s picture

Is there an update on when the function for making this configurable is available?

smustgrave’s picture

Not sure a configurable message is the current proposal but more just a potentially better message

Least based on the current issue summary.

oily’s picture

RE: #20 and @quietone's proposed message

Something went wrong. Your browser's developer console contains more details. Contact the site administrator if the problem persists.

I don't like the 'Contact the site administrator if the problem persists.' It is too generic. There are lots and losts of other core error messages that you could append that sentence to. And IMO it would not be helpful. It sounds a little opinionated. 'Oops' is a little patronising at the other end of the scale. Opinionated in the sense that although every Drupal site has a user 1 'Site administrator' account, there will be plenty of organisations using Drupal who do not have a clear IT hierarchy with an identifiable 'Site administrator.'

It would be interesting to do a survey but I would guess that in a typical charity or SME business the whereabouts and identity of the 'Site administrator' might be unknown if not unknowable. There will often be several possible 'Web savvy Samantha or Bobs' or maybe the guy at the next desk.

I think the message should be pitched at those who know what browser developer tools are and how to access the console. If they dont know then we can assume they will reach out to others for help without our suggesting it.

Taking that approach, pitching the message at the savvy user, I think it reasonable to include the term 'AJAX' in the message. Let's not avoid it like it is a 'dirty word' that will cause fear and alarm! And remember there is google!

The current error message I agree is bad. It sounds like 'Something went wrong, but it's way above your head, so I'm not even going to try to explain it to you...'

oily’s picture

RE #19 and #20, @james.williams, Rather than involving locale module and translations, if just the one message is to be overridden it seems the cory jquery file responsible can be overridden in the theme within a custom theme library. something like this?

larowlan’s picture

Status: Needs review » Needs work

There's nothing to review here now that the direction has changed. I think our next step is to propose and agree on a better default.

oily’s picture

Perhaps

An Ajax-related error has occurred. Check your browser's console for further details.

s_leu’s picture

I recently looked at a way on how to expose access denied messages a bit better instead of simply showing the "Oops" message and found this issue here.

Looking at the MR i think it's a good start and I had some ideas on how to improve this a bit further. Here some proposals:

  1. we can actually make this a) more useful for developers by putting whatever is contained inside xmlhttp.responseText into the message instead of the hardcoded "Oops" or the configurable message from the MR above. That would be a nice DX improvement, it'd enable instant view of backtraces and error messages in the message instead of having to look that up in the console after an ajax error. This should probably only be happening when $config['system.logging']['error_level'] is set to verbose
  2. As for cases where the "Oops" message is shown after an access denied exception inside an ajax call, we could possibly display access denied reasons based on the reason specified in an instance of AccessResultReasonInterface. This should probably be configurable as well.
  3. Whenever an ajax error message is displayed it would probably make sense to scroll the window to the message as otherwise it's easy to miss on pages with a height that's higher than the viewport.

Adding a PoC patch that addresses the first two points here for reference. It's definitely not ready for production but could possibly serve as a base for making further improvements before merging the MR here. Note the todos and also that the patch is against 10.3.x.

coffeymachine’s picture

IMO, AJAX error messaging could use a few improvements:

  1. Echoing @s_leu, having the option to put the xmlhttp.responseText would be immensely helpful.
  2. AJAX callbacks need to be able to act on PHP exceptions. Consider when Guzzle returns an API error response. The go-to behavior is to throw an exception and log it to the console. AJAX callbacks need a way to translate these error responses into human-readable messages. Perhaps we should look at the ExceptionLoggingSubscriber class to see if it's feasible to add a hook or some other means of giving the module developer the ability to act on an exception thrown by another module.
shmy’s picture

I believe the #32 patch would improve the current behavior.

Just would like to propose to allow customization through JS.
Currently i'm using something like the following hack to prevent displaying core's message and instead providing a context related one, or none at all (e.g. on AJAX calls that aren't triggered by user interactions).

Drupal.AjaxError.messages = {};
Drupal.AjaxError.messages.add = () => {};

A clean solution, e.g. a param or setting, would be much better IMO.

shmy’s picture

Why don't we support declaring error handling on a render array, where the context is available? In my eyes that would improve the DX and eventually UX.
A default error handler could be provided that varies the messages even on the same status code (as mentioned in #9).

E.g.:

[
  '#ajax' => [
    // Default error handler
    'on_error' => [
      MyClass::ajaxErrorCallback,
    ],
    // Override for any 5xx response
    'on_5xx' => [
      // A list AJAX commands.
      new MessageCommand($this->t('We are unable to procced your message right now. Please try again later.')),
    ],
    // Override for a 408 (timeout) 
    'on_408' => [
      new MessageCommand($this->t('Your message was not sent. Please try again.')),
      // A custom command.
      new NotifyThirdPartyServiceCommand(...$params),
    ],
  ],
];

I'm aware that i'm stretching the scope of the issue by lot. But something like that could eliminate the need for custom JS in some cases.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

catch’s picture

StatusFileSize
new248.71 KB

Toot from Yvan on Mastodon with a screenshot of an AJAX error on the website of the British wildlife trusts, with the commentary. LOL, whut? Check your browser's developer console for more details. I'm sure that isn't just entirely confusing to most people. Not at all. - screenshot has a large, themed Drupal error message saying Oops something went wrong, check your browser's developer console for more details

Uploading a screenshot from mastodon with permission - someone ran into this as a site visitor on their local wildlife trust website, and thought it was confusing enough to post it on mastodon.

I think we should consider changing the behaviour so that this only shows when system.logging.error_level is display, which might mean putting the value of that into Drupal.settings for ajax.js to read.

Also after typing this, wondering what HTMX does for this.

catch’s picture

StatusFileSize
new127.02 KB

edit: better screenshot.

rkoller’s picture

In regard to error messages i tend to cite the three principles from the "Writing is Designing" book by Michael Metts and Andy Welfle on Rosenfeld Media, which are also a good benchmark evaluating potential proposed solutions:

Avoid - Find way to help your user without showing them an error
Explain - Tell your users what is going on and what went wrong
Resolve - Provide a solution to the problem that the user is facing

The current state doesn't meet the avoid and resolve principle, and the explain principle is also not necessarily met, cuz the user not necessarily understands the error message in the "developer console" the error message refers to, if they know where to find the developer console in the first place. But skimming through the comments so far it sounds like the error isnt triggered by one root cause?

fathershawn’s picture

Making a note that we are not currently communicating anything to the site user about ajax request failures via htmx. We can do so by listening for the htmx:afterRequest event and checking the event data for an error.

aporie’s picture

In my opinion. And not because I had personal argues with Catch. I actually do agree with him.

We should restrict this feature to a system.logging.error_level.

Developers should be able to handle ajax errors as they see fit and it would actually be an added value to add a developer friendly message for debugging purpose.

But surely not to display it to end users (or at least to rephrase it). Though, I again sign for #9. This generic message shouldn't be intrusive for end users to let developers deal as they see fit with the error handling. Not all errors are not predicted. Actually, most of them should be (predictable, otherwise your application has a bug).

fathershawn’s picture

As we move to HTMX, that's essentially where we are unless we do something @aporie, although the current ajax setup stems from an issue that asked for the opposite! Anyone in their code could listen, and respond to errors. I haven't yet gone to read the prior issue and the reasoning for what we have.

We could in theory code an event responder clientside that triggers a specific error message event that contrib could override or suppress.

system.logging.error_level though makes me think of server-side loggers. Isn't this issue about clientside error message? (I'm still catching up!)

aporie’s picture

We could in theory code an event responder clientside that triggers a specific error message event that contrib could override or suppress.

I'm not up to date with HTMX, but if clientside we can catch this error and make it our own, then I'm all for it.

Thing is, not to throw a random error to clients when we haven't dealt with it. Every error should be handled correctly. Now, if you have a way to create a generic message for errors that end users can understand as simple (an error occured) and which developers can read (here is the backtrace). I'm all in for it.

lauriii’s picture

The problem with restricting this to the logging level is that showing no error message at all is probably the worst UX outcome. When something fails silently, the user has no idea anything went wrong. Instead, the form just doesn't respond or the page doesn't update. That's more confusing than a generic message, even if it's imperfect. With the imperfect message the user at least knows that they can stop waiting and they have to try again. The issue where this was added tried to solve this.

What I think we should focus on instead is making it easier for developers to provide better error messages that are contextual. The current generic message is a reasonable fallback, but the real improvement would be an API that lets developers define meaningful error responses for their AJAX interactions. This could be something along the lines of what's proposed in #35.

aporie’s picture

@laurii

I totally agree with you. This is the worst:

When something fails silently, the user has no idea anything went wrong.

I don't think we should hide errors silently. I think we should make a generic message for users to report it. And ... If we can. Give more details to developers. I got it. The message, check in your console was about helping developers. But that's After Purchase Service.

When you deal with websites the Drupal project is dealing with, you want to bring confidence, not debugging outputs. "Check your console" is like: we're a government asking you for help ^^.

If you're a government, you can definitely afford me just telling you: "I had the error message: Something went wrong!".

What do we expect? From anyone to be a developer? Well with AI, this might come true.

aporie’s picture

In addition, I will add that any ajax error message which is not handled from the app perspective is a bug.

There are plenty ways of sending errors to an ajax request and most of them should be calculated.

Sending a random error message, quite alarming? for non techy, is a mistake to me.

I personally send tons of 403 and 404 from my controllers. I don't want my users to see: "Oops, something went wrong. Check your console for more details!".

I want to control why they are restricted to this area, still displaying the buttons and giving them a chance to think they are free :). (this is obvioulsy more a business approach than a governmental one, we all use Drupal for different purposes).

dries’s picture

I think this issue is going in the right direction.

Differentiating the message by HTTP status code has been suggested several times (@glugmeister in #7, @aporie in #9, @shmy in #35). I think that is the right approach.

We just need to agree on the language, and the Avoid / Explain / Resolve framework from #39 can help. Helpful recommendation @rkoller!

I'd propose one template with the error code as a parenthetical:

  • "Something went wrong (error 500). Please try again later."
  • "Something went wrong (error 403). Please try again later."
  • "Something went wrong (network error). Please try again later."

For end users, all error messages boil down to "it didn't work, try again later". I went round and round, and I think that is the most honest and helpful a generic message can be.

The status code differentiation is useful for developers debugging the issue, or for end users reporting the problem to a site owner. So I think it is valuable to include, in a way that does not confuse or frustrate end users.

In short, the error code is ignorable by end users but reportable. I've seen this same pattern used on other websites.

A couple of people suggested adding "Contact the site administrator if the problem persists" (@quietone in #2). But as @oily noted in #28, not every site has an identifiable administrator. If CNN.com was built on Drupal, for example, that would be a rather odd message for readers of CNN.com. With the error code included, users already have something concrete to report if they decide to escalate through a contact form or support email address they can find on the site.

Testing the proposed language against the framework in #39:

  • Avoid: can't avoid showing the error, but the code is designed to be ignorable.
  • Explain: "Something went wrong" is universally understood. The error code/text between parenthesis adds precision for those who can use it.
  • Resolve: "Please try again later" is honest and actionable. Users have to wait for network problems to get resolved, 403/404 or 500 errors to be fixed, etc. We don't know how long they have to wait or when to retry.

Compared to the current "Oops, something went wrong. Check your browser's developer console for more details":

  • Removes "Oops" (potentially patronizing on professional sites, as noted in #20).
  • Removes "developer console" (confusing to end users, as the screenshot in #37 illustrates).
  • Adds "Please try again later" (actionable).
  • Adds the error code (useful for reporting).

For developers who want contextual messages (e.g. "You don't have permission" for 403, or "This item no longer exists" for 404), the developer API proposed by @lauriii in #44 and @shmy in #35 could be a great follow-up. In the mean time, the error code would be in the new error message, which is already a huge improvement for developers.

This may not be perfect, but it's clearly better than what we have. Personally, I'd be very happy to see this land after 3 years (and so would the person in catch's screenshot). :-)

dries’s picture

Maybe something like this?

lauriii’s picture

Status: Needs work » Needs review

The changes looked good. I was writing an identical MR (with the help of Cursor). I added additional tests that I had created to the MR.

catch’s picture

I think we should add a check that the status code is >=400 before showing it in the message, so we don't end up showing Error: 200 - thinking about HTML or other 200 but invalid responses, infinite redirects, maybe a strangely implemented bot protection layer, things like that.

The wording for the message is good, and I think this is better than my idea to base it on error logging - we do need to tell people something went wrong, just not to learn how to use their developer console at the same time.

rkoller’s picture

The message is an improvement compared to the previous. The only problem I see, in the context of the question how to resolve the error. What if the error doesn't dissolve? The recommendation how to proceed is to try again later. But if I try now in five or ten minutes and the issue persists, and then wait another half an hour and the error still persists - the try again later recommendation sends that person in an indefinite infinite loop. So the person is in the limbo between will the error vanish in one future reload of the page or will the error show up - instructions how to "fix" the problem and a way forward is not necessarily provided. As a none technical person I word be lost still nevertheless.

aporie’s picture

I agree that the wording "Something went wrong (error 500). Please try again later." is better, neutral, and universal like Dries said.

So the person is in the limbo between will the error vanish in one future reload of the page or will the error show up

:D I was actually always struck by that too, until you understand with experience that these error messages are generic and mean: "it's not a temporary error, but we'll be working on it soon".

Normally all these errors end up in the watchdog right? So eventually if developers are checking it often to see potential active bugs they will find it with the associated backtrace whether the users report it or not.

Now to create a message, generic enough, to say "Something went wrong (error 500). It'll get fixed when we figure it out :D" without saying it as is. I don't know. Let's see what AI can come up with.

So I got:

1. The "Contact Support" Approach (Best for Internal/B2B)
"An unexpected error occurred. Please refresh the page or try again in a few minutes. If the issue persists, please [contact support / report this issue]."

2. The "Action-Oriented" Neutrality
"We're having trouble processing this request (Error 500). Please check your connection and try again. If the problem continues, the service may be temporarily unavailable."

3. The "Soft Technical" Hybrid
"Something went wrong on our end. Please try again shortly. If you continue to see this message, please try again later or visit our [Status Page/Help Center]."

Which again brings us to "support" problem or the "Page help center" which is totally too specific to be a generic error for Drupal. That makes the 2 ok. It's a bit long...

Something like: "Something went wrong (error 500). Please try again later. If the problem continues, the service may be temporarily unavailable."

Regarding the MR it looks good but I haven't tested it. Maybe tomorrow.

I want to add something about my #9. Because from what I recall I had an issue in a specific scenario (but that was a long time ago so I might be totally wrong):

It was an auction site where users have credits. They can bid on auctions with a simple form to enter an amount and a button to bid, which calls an ajax callback in a service (and not using "use-ajax" but fetch in a js file). The thing is this callback may return several differents error. Again, it was a while ago so I'm gonna make some up:

1 - Not enough credits (403)
2 - Your daily limit of bid has been reached (429)
3 - Your bid is under the current higher bid (409)
etc etc.

From what I can tell from my patch in #6 (and my previous comment) it's that I targetted only 500 to make sure the generic message wouldn't be appended to my JsonResponse message.

Still the problem we are creating is that in my case above the user will see:
Not enough credits
Something went wrong (error 403). Please try again later.

But again it's from memory. I will try to reproduce it tomorrow to see if what I'm saying is what is happening.

dries’s picture

I think "If the problem continues, the service may be temporarily unavailable" makes the message longer without adding much. Personally, the shorter version felt better to me: "Something went wrong (error 500). Please try again."

And @catch's suggestion in #50 about checking status >= 400 still needs to be implemented. I think that is a good idea.

rkoller’s picture

so when one of these ajax errors happens it is ensured that the problem resolves itself on one of the subsequent reloads without any additional manual intervention by the user?

dries’s picture

You're right that "try again" doesn't guarantee the error goes away. Bugs don't fix themselves no matter how many times you retry.

But in practice, people don't get stuck in an infinite loop. They try once or twice, and if it persists, they naturally come back later or reach out for help.

That is just my 2 cents. Let's see what others think.

rkoller’s picture

would it be possible to categorize the different errors based on the necessary measure how to resolve them or does every error need a different type of intervention? i don't think you need precise stepwise instructions how to resolve an error (would go beyond the scope of an error message) but to provide a rough pointer what kind of measure is necessary might be helpful - just to prevent that the user feels lost.

rkoller’s picture

and on a side note referring to the current MR, the word please should be omitted per https://www.drupal.org/docs/develop/user-interface-standards/interface-t...

amateescu’s picture

Status: Needs review » Needs work

Posted a few comments on the MR.

dries’s picture

Re #58: Categorizing errors sounds appealing, but the same error code can mean very different things.

A 403 could be "you need to log in", "you don't have permission", "your subscription expired", etc. They each have different resolutions.

A 500 could be a temporary server problem or a persistent bug in the code only a developer can fix.

As a result, we don't know what the best next step is.

A developer API for contextual error messages (@lauriii #44, @shmy #35) could solve this by letting developers pass specific guidance for their use cases. That would be a great follow-up.

dries’s picture

I addressed the review feedback from @amateescu and @catch:

  1. Removed 'Please' per Drupal interface text guidelines.
  2. Added return types and @throws docs to test controller methods.
  3. Restored the comment explaining why we reset drupalActiveXhrCount.
  4. Removed cspell comments.
  5. Only show numeric status code for >= 400. Status 0 shows 'network error'. Non-error status codes (e.g. 200 with invalid response), omit the code entirely per @catch.
dries’s picture

Status: Needs work » Needs review
aporie’s picture

StatusFileSize
new5.86 KB

Sorry, I just figured I got messages from you here. I was focused on trying to get the pipeline green.

So we don't validate the "If the problem continues, the service may be temporarily unavailable"? It seemed to me a nice addition as per #52.

There is still a cspell issue indeed, because of the "Please". I personally don't see much problem to it, because the sentence is "universal" and users are used to it. But if we want to remove it, then we are back to our wording ...

Also, regarding my #53 I vibe coded an attempt to reproduce it. And I can't, so either my memories are wrong, either it just disappeared. I'm attaching the patch (dirty) for reference.

[EDIT] Also, we have a false positive in core/tests/Drupal/Tests/PerformanceTestTrait.php:685, maybe we could just increase a bit the "range" from 500 to 600, but TBH I don't know what it impacts really, I guess the expected size of the script trying to add a page in umami profile. It's not something I'm confident with.

rkoller’s picture

re #60 hmmm so even a single returned error code can have different kinds of resolutions depending on the root cause of the error? that makes things even more tricky. :/ but as a user it would be already tremendously helpful to have the additional context to an error code available like exemplified with:

A 403 could be "you need to log in", "you don't have permission", "your subscription expired", etc. They each have different resolutions.

A 500 could be a temporary server problem or a persistent bug in the code only a developer can fix.

That way, the user knows for example with error code 403 that they dont have the necessary permissions or that their subscription expired; that helps understanding the error significantly and empowers them to even solve the problem on their own or at least they know whom to address next for help. the solution might be out of scope for this issue alone. but if it would be possible to have a list of possible http errors (>= error code 400) and the list of potential root cause for each of those http errors some customized error messages could be written. we could discuss and come up with a draft in the ux meetings on friday for example.

catch’s picture

@rkoller that's very hard to do and would need its own issue.

If a user visits /admin and gets a 403, we have no idea if they could access it if they were logged in, or if it would still be a 403, or if they have an account at all, or even if they're able to create an account on the site. Telling them to log in when they're logged out might be misdirection.

Yet another possibility especially with AJAX is the site builder forgot to give a permission required for the AJAX route to users who have access to the UI that makes the request. So they might be able to start accessing it once given an extra role or if their existing role gets and extra permission. But equally maybe they've been given access to the UI by accident and that will go away.

dries’s picture

Challenge accepted. I'm not really an expert on writing JavaScript tests but I added one. Full disclosure: I used Claude Code for this, but it verified it manually, and it follows the exact same pattern as the network error test that lauriii added. Tests pass locally and the code makes sense, so I think this is good!

lauriii’s picture

Status: Needs review » Reviewed & tested by the community

Looks great! Thank you @dries!

fathershawn’s picture

Status: Reviewed & tested by the community » Needs work

This issue expands the sized of ajax.js so core/profiles/demo_umami/tests/src/FunctionalJavascript/AssetAggregationAcrossPagesTest.php:110 needs to be adjusted.

dries’s picture

Status: Needs work » Reviewed & tested by the community

Fixed the ScriptBytes assertion. Setting back to RTBC.

catch’s picture

Status: Reviewed & tested by the community » Needs work

Error message logic looks good to me now. A couple of questions on the test.

dries’s picture

Tried to address catch's feedback on the tests.

dries’s picture

Status: Needs work » Needs review

Also rebased it. Tests seem to pass. Not entirely sure if I should set it to RTBC or 'Needs review'.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.