Skip to content

Comments

Improve GraphQL error handling by mapping all DataApiBuilderException types to appropriate HTTP status codes#2692

Draft
Copilot wants to merge 3 commits intomainfrom
copilot/fix-2691
Draft

Improve GraphQL error handling by mapping all DataApiBuilderException types to appropriate HTTP status codes#2692
Copilot wants to merge 3 commits intomainfrom
copilot/fix-2691

Conversation

Copy link
Contributor

Copilot AI commented May 19, 2025

Why make this change?

Issue #2691 requests that all DataApiBuilderException.SubStatusCodes should be inspected to determine the correct HTTP status code for GraphQL responses, rather than only handling DatabaseInputError. Previously, the serializer would return BadRequest (400) for database input errors but default to server error (500) for all other exceptions, making it difficult for clients to distinguish between different error types.

What is this change?

This PR enhances the DabGraphQLResultSerializer class to properly map all types of DataApiBuilderException.SubStatusCodes to their appropriate HTTP status codes in GraphQL responses using a clean switch expression pattern.

The serializer now maps:

  1. Authentication errors to 401 Unauthorized
  2. Authorization errors to 403 Forbidden
  3. Entity/item not found errors to 404 NotFound
  4. Input validation errors to 400 BadRequest
  5. Unsupported features to 501 NotImplemented
  6. Conflict errors to 409 Conflict
  7. Server-side errors to 500 InternalServerError

Implementation Details:

  • Refactored status code mapping using idiomatic C# switch expression for cleaner, more maintainable code
  • Created separate MapSubStatusCodeToHttpStatusCode method with pattern matching
  • Updated class documentation to clearly explain the purpose and behavior
  • Comprehensive unit test suite with 31 tests covering all mappings and edge cases

This implementation ensures that GraphQL responses have the appropriate HTTP status codes that match the nature of the error, which improves API consistency and allows clients to better handle different error types.

How was this tested?

  • Unit Tests

Test Coverage:

  • 31 unit tests in DabGraphQLResultSerializerTests.cs
  • All 25 SubStatusCode enum values mapped and tested
  • Edge cases: null errors, empty error lists, invalid error codes, multiple errors, non-query results
  • All tests passing ✓
  • Code review: No issues ✓
  • Security scan (CodeQL): No vulnerabilities ✓

Sample Request(s)

When a GraphQL query encounters an authorization error:

query {
  book_by_pk(id: 1) {
    title
  }
}

Before: HTTP 500 Internal Server Error
After: HTTP 403 Forbidden (when AuthorizationCheckFailed SubStatusCode is set)

When a GraphQL query references a non-existent entity:

query {
  nonexistent_entity {
    id
  }
}

Before: HTTP 500 Internal Server Error
After: HTTP 404 Not Found (when EntityNotFound SubStatusCode is set)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…LResultSerializer

Co-authored-by: Aniruddh25 <3513779+Aniruddh25@users.noreply.github.com>
Copilot AI changed the title [WIP] Inspect all DataApiBuilderExceptions to determine the correct status code of HotChocolate responses Improve GraphQL error handling by mapping all DataApiBuilderException types to appropriate HTTP status codes May 19, 2025
Copilot AI requested a review from Aniruddh25 May 19, 2025 20:06
@souvikghosh04
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@JerryNixon JerryNixon added the 2.0 label Feb 4, 2026
@JerryNixon

This comment was marked as resolved.

…ve unit tests

Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 16, 2026

@copilot

🔴 1. Critical: The PR modifies a file that no longer exists
...

I've addressed the actionable feedback from your review in commit 7c04624:

#3 Tests Added: Created comprehensive unit test suite with 31 tests covering:

  • All SubStatusCode→HTTP status mappings (401, 403, 404, 400, 501, 409, 500)
  • Edge cases (null errors, empty list, invalid codes, multiple errors, non-query results)
  • All tests pass successfully

#4 Switch Expression: Refactored to use idiomatic C# switch expression with pattern matching for cleaner, more maintainable code

Regarding #1 and #2:

  • DetermineStatusCodeMiddleware.cs doesn't exist in the current codebase (base commit 7f8fa6b). DabGraphQLResultSerializer.cs is the file present and registered in Startup.cs:382.
  • GlobalMcpEndpointDisabled enum value doesn't exist in SubStatusCodes yet. The switch expression's default case will safely handle it as 500 when added.

If there's a newer main branch or reference implementation showing the middleware approach, please share the branch name so I can rebase and adapt the changes accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants