-
-
Notifications
You must be signed in to change notification settings - Fork 562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix GraphQL IDE charset error #3824
Conversation
Why: - Hide non-printing characters This change addresses the need by: - Explicitly setting the content-type charset to utf-8
Reviewer's Guide by SourceryThis pull request fixes a character encoding issue in the GraphQL IDE by explicitly setting the content type charset to utf-8. This ensures that non-printing characters are properly handled. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @moritz89 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Thanks for fixing this bug, explicitly setting the charset is the right approach.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR updates the async GraphQL IDE in http_handler.py to explicitly set the content-type header to "text/html; charset=utf-8", addressing formatting errors seen in GraphiQL (#3821).
- In strawberry/channels/handlers/http_handler.py, render_graphql_ide() now returns "text/html; charset=utf-8" for async IDE responses.
- The change resolves hidden character issues in GraphiQL without impacting other functionalities, though sync consumers remain unchanged.
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3824 +/- ##
=======================================
Coverage 95.24% 95.24%
=======================================
Files 499 499
Lines 32417 32417
Branches 1681 1681
=======================================
Hits 30877 30877
Misses 1275 1275
Partials 265 265 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #3824 will not alter performanceComparing Summary
|
Updated MR to handle sync consumer as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
This PR fixes the GraphQL IDE charset issue in http_handler.py by explicitly setting the content-type header to "text/html; charset=utf-8", eliminating hidden characters in GraphiQL.
- Modified /strawberry/channels/handlers/http_handler.py: render_graphql_ide now returns "text/html; charset=utf-8".
- Applies change consistently for both async and sync GraphQLHTTPConsumer handlers.
- Resolves the formatting error reported in issue Partial content-type results in GraphiQL formatting errors #3821 with minimal impact on existing functionality.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you so much!
do you want to add a RELEASE.md file so we ship this? :D
* Fix GraphQL IDE charset error Why: - Hide non-printing characters This change addresses the need by: - Explicitly setting the content-type charset to utf-8 * Include sync http consumer * Add release file * Update RELEASE.md --------- Co-authored-by: Patrick Arminio <patrick.arminio@gmail.com>
Description
Why:
This change addresses the need by:
Types of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by Sourcery
Bug Fixes: