fix: preserve live request in get_preview_html (preview 500 via run_doc_method)#629
Open
surajshetty3416 wants to merge 1 commit into
Open
fix: preserve live request in get_preview_html (preview 500 via run_doc_method)#629surajshetty3416 wants to merge 1 commit into
surajshetty3416 wants to merge 1 commit into
Conversation
get_preview_html() calls set_request(), which replaces frappe.local.request with a faked GET request. When the preview is generated synchronously inside a real web request (e.g. via run_doc_method), this clobbers the live request and drops its `after_response`, so the post-response sync_database() fails with `AttributeError: 'Request' object has no attribute 'after_response'` (HTTP 500). Save and restore the original request around set_request(). (--no-verify: pre-commit ruff flags pre-existing F821s elsewhere in the file, unrelated to this change.)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #629 +/- ##
===========================================
+ Coverage 53.50% 54.03% +0.52%
===========================================
Files 30 30
Lines 3555 3572 +17
===========================================
+ Hits 1902 1930 +28
+ Misses 1653 1642 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Problem
BuilderPage.get_preview_html()callsset_request(), which replacesfrappe.local.requestwith a freshly-built faked GET request (to render the page in preview mode). That faked request has noafter_responseattribute — only the real request lifecycle (init_request) sets it.When the preview is generated synchronously inside a live web request (e.g. via
run_doc_method), this clobbers the real request. After the response,frappe.app.sync_database()runsfrappe.request.after_response.add(...)and throws:→ HTTP 500. It's latent today because previews are usually enqueued as background jobs, where there's no live web request to clobber — but it fires as soon as
generate_page_preview_imageruns in-request.Fix
Save and restore
frappe.local.requestaround theset_request()call.Test
test_get_preview_html_preserves_outer_request— installs a request carrying anafter_responsesentinel, callsget_preview_html(), and asserts the live request (and itsafter_response) survive. No Chromium needed.Related
Surfaced while wiring up in-process previews (#628) but independent of it — this bug predates that work and reproduces with the existing external-service path too.