Skip to content

Refactor(index.php): entrypoint error handling and improve fatal fallback messaging#60362

Open
joshtrichards wants to merge 2 commits into
masterfrom
jtr/refactor-index-entrypoint-error-handling-and-messaging
Open

Refactor(index.php): entrypoint error handling and improve fatal fallback messaging#60362
joshtrichards wants to merge 2 commits into
masterfrom
jtr/refactor-index-entrypoint-error-handling-and-messaging

Conversation

@joshtrichards

@joshtrichards joshtrichards commented May 13, 2026

Copy link
Copy Markdown
Member

Summary

This changeset refactors index.php error handling to make the different failure paths easier to follow and more consistent.

What changed

  • Centralized the non-HTML error response logic for LoginException and MaxDelayReached
  • Reworked inline comments to better describe:
    • expected client-side failures
    • server-side failures that may require admin attention
    • the nested fallback tiers used when error rendering itself fails
  • Improved the final plain-text fatal-error fallback with clearer user guidance
  • Added a timestamp to the fatal fallback output to help with support correlation when a request ID is unavailable
  • Preserved the original fatal error propagation with throw $ex;

Notes

  • The observable behavior for the main exception paths remains the same
  • The fatal-error fallback text has been updated for clarity
  • The non-HTML response handling is centralized in a helper for readability and consistency

Follow-up considerations

We should consider:

  • whether some of the other error code paths should have non-HTML/JSON response handling (didn't change that since it would be a material behavior change)
  • making the Accept header check more robust to avoid false positives, including exact string matching as well as proper handling of quality factors (q=)

TODO

  • Possibly update tests (only if some check the actual text output)
  • Test test test

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@joshtrichards joshtrichards added 2. developing Work in progress ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels May 13, 2026
@joshtrichards joshtrichards changed the title Refactor index.php entrypoing error handling and improve fatal fallback messaging Refactor(index.php): entrypoint error handling and improve fatal fallback messaging May 13, 2026
@joshtrichards joshtrichards added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 13, 2026
@joshtrichards joshtrichards marked this pull request as ready for review May 13, 2026 21:53
@joshtrichards joshtrichards requested a review from a team as a code owner May 13, 2026 21:53
@joshtrichards joshtrichards requested review from CarlSchwan, artonge, leftybournes and provokateurin and removed request for a team May 13, 2026 21:53
…atal fallback messaging

Move the repeated LoginException/MaxDelayReached non-HTML response logic into a shared helper so the request error handling reads more consistently. Rework the inline comments to better describe the different failure tiers and distinguish expected client-side errors from server-side issues that may require admin attention.

Also improve the final fatal-error plain-text fallback so it provides clearer guidance for users and administrators, including a timestamp for support correlation (all we can rely on in the last resort path) and dropping the erroneous prior inline request to  "include the technical details below in your report" (there aren't any), while preserving the original error propagation.

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards force-pushed the jtr/refactor-index-entrypoint-error-handling-and-messaging branch from 3cabc5d to 2c9da5c Compare May 14, 2026 16:47
Comment thread index.php
Comment on lines +43 to +46
exit();
}

$templateManager->printErrorPage($message, $message, $statusCode);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exit call is new, right?
I would have thought that the following would be closer to the original.

Suggested change
exit();
}
$templateManager->printErrorPage($message, $message, $statusCode);
else {
$templateManager->printErrorPage($message, $message, $statusCode);
}

@susnux susnux added the community pull requests from community label Jun 9, 2026
@susnux susnux added this to the Nextcloud 35 milestone Jun 9, 2026
@come-nc come-nc added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress community pull requests from community ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants