Take the server's error page back to the landing screen - #588
Merged
Conversation
The core translates a page on the server thread, long after CoreLoader reported success, so a rendering failure only reaches the app as a status code. onReceivedError never sees one - the server did answer - so nobody detected it and the WebView rendered odrcore's "Internal Server Error" page in place of the document. PageView now overrides onReceivedHttpError as well, and both it and onReceivedError take the main frame through failPage(). DocumentFragment ends it where a document that would not open ends: back on the landing screen with the contact dialog. This also covers a server that never bound, which comes back as ERR_CONNECTION_REFUSED. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EVNWZNEjtiodTCP8kvTfNP
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32709b04f9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
shouldOverrideUrlLoading returns false when it finds no app for a link, which leaves the webview to try the navigation itself - and that failure arrives as a main frame error too. failPage() now only gives up when the url is our own content, so a dead hyperlink no longer presents the open document as damaged. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EVNWZNEjtiodTCP8kvTfNP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A user sent a screenshot of the viewer showing nothing but Internal Server Error. That string is cpp-httplib's 500 page, shipped inside
libodr_jni.so- the WebView was rendering the local server's error body as if it were the document.Why nothing caught it
The core translates a page on the server thread, long after
CoreLoader.render()reported success, so whatever it throws there only reaches the app as a status code.PageViewoverrodeonReceivedError, which never fires for an HTTP status - the server did answer. So the failure was neither logged nor shown.What this does
PageViewoverridesonReceivedHttpErrortoo. Both it andonReceivedErrortake a main-frame failure throughfailPage(), which logs toCrashManagerand calls the fragment back.DocumentFragment.onPageFailed()unloads the document, drops back to the landing screen and raises the existingdialog_broken_file- the same "the file may be damaged, or something went wrong on our side" offer a file that will not open at all gets. Guarded so a second view, or a page failing on the way out, is a no-op, and deferred throughreplayOnStartwhile the activity is stopped.page_failedalongside the existingclose_failed_document.Routing
onReceivedErrorthe same way also covers a server that never bound: that comes back asERR_CONNECTION_REFUSEDand used to leave Chrome's error page on screen.Verified
aPageTheServerCannotServeOffersContactopenstest.odt, then points the page at a path the server has nothing under. On a Pixel 6 Pro emulator the path is exactly the one the bug takes:Full instrumented suite 68/68, plus
spotlessCheck,testProDebugUnitTest,lintProDebugandassembleDebug.🤖 Generated with Claude Code