-
Notifications
You must be signed in to change notification settings - Fork 3
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): Set max execution depth to allow inspection query #1679
fix(graphql): Set max execution depth to allow inspection query #1679
Conversation
📝 WalkthroughWalkthroughThe pull request involves a configuration change in the GraphQL service setup for the Dialogporten project. Specifically, the maximum execution depth for GraphQL queries has been increased from 10 to 12 in the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Digdir.Domain.Dialogporten.GraphQL/ServiceCollectionExtensions.cs (1)
28-28
: Consider monitoring query depth patterns.To ensure this depth limit remains appropriate:
- Monitor for queries approaching the depth limit
- Watch for performance impacts from deeper queries
Consider implementing query depth monitoring in your telemetry:
- Log queries that exceed 80% of max depth (>9 levels)
- Track query depth distribution metrics
- Set up alerts for repeated deep query patterns
This will help validate whether 12 is the optimal depth limit for your use case.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Digdir.Domain.Dialogporten.GraphQL/ServiceCollectionExtensions.cs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build / build-and-test
🔇 Additional comments (1)
src/Digdir.Domain.Dialogporten.GraphQL/ServiceCollectionExtensions.cs (1)
28-28
: LGTM! Consider adding documentation for the depth requirement.The increase in max execution depth from 10 to 12 is a reasonable adjustment to accommodate the inspection query.
Consider adding a code comment explaining why this specific depth is required, to prevent future accidental modifications:
+ // Depth of 12 is required to support GraphQL inspection queries .AddMaxExecutionDepthRule(12)
Let's verify the depth requirements of your GraphQL queries:
✅ Verification successful
Depth increase to 12 is justified by schema complexity
The schema reveals legitimate deep nesting patterns, and inspection queries need to traverse the entire type system to expose schema information. The previous limit of 10 was set primarily for
subParties
recursion without considering inspection queries.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find GraphQL queries to analyze their depth requirements # Look for .graphql files or GraphQL queries in code rg -t graphql . || rg 'gql`[^`]*`|"""[^"]*"""' -g '!node_modules' # Look for any existing documentation about query depth rg -i 'depth|nested|graphql.*(limit|max)' -g '*.md'Length of output: 440
Script:
#!/bin/bash # Search for GraphQL queries in test files and any GraphQL schema definitions rg -l '(query|type|interface|input|schema).*{' --type-add 'test:*.{spec,test}.*' -t test # Try to find any GraphQL related files fd -e graphql -e gql # Use GitHub CLI to fetch issue content gh issue view 1430 || true gh issue view 1431 || trueLength of output: 2838
Script:
#!/bin/bash # Check the schema file content cat docs/schema/V1/schema.verified.graphql # Look for recent changes to the schema file git log -p docs/schema/V1/schema.verified.graphql | head -n 100Length of output: 9562
🤖 I have created a release *beep* *boop* --- ## [1.45.0](v1.44.2...v1.45.0) (2025-01-15) ### Features * added id to attachments, ApiActions and GuiActions in DialogCreate ([#1670](#1670)) ([470e5a9](470e5a9)) * **apps:** export logs to open telemetry endpoint ([#1617](#1617)) ([1a71763](1a71763)) * **janitor:** add otlp logger for janitor ([#1686](#1686)) ([2e1656b](2e1656b)) ### Bug Fixes * **app:** Add missing telemetry setup GraphQL and Service ([#1695](#1695)) ([601a826](601a826)) * Authentication level claim is 0 in dialog token ([#1654](#1654)) ([37e545a](37e545a)) * **graphql:** Add missing activity types ([#1684](#1684)) ([a0697ae](a0697ae)) * **graphql:** Set max execution depth to allow inspection query ([#1679](#1679)) ([6265110](6265110)), closes [#1680](#1680) * **web-api:** re-enable health checks ([#1681](#1681)) ([96c2c3e](96c2c3e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Missing some depth to allow the inspection query through
Issue #1680