Skip to content
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

Ability to add errors to resolved fields #512

Merged

Conversation

VectorTetra
Copy link
Contributor

@VectorTetra VectorTetra commented Feb 4, 2025

Some time ago @xperiandri removed this functionality as exceptions were no longer processed anywhere.
Now I add it back aligned to the new Result based internal resolvers execution

Covered test case: Add additional error and return value from a non-nullable GraphQL field resolver
@VectorTetra VectorTetra force-pushed the add-errors-to-resolved-fields branch from 6ce4185 to 8c5a23c Compare February 4, 2025 15:02
src/FSharp.Data.GraphQL.Server/Execution.fs Outdated Show resolved Hide resolved
match ctx.Context.Errors.TryGetValue ctx with
| true, errors ->
errors
|> Seq.map (GQLProblemDetails.OfFieldExecutionError (path |> List.rev))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You either do List.rev inside function OfFieldExecutionError or at least create function normalizedPath() = path |> List.rev somewhere

src/FSharp.Data.GraphQL.Shared/TypeSystem.fs Show resolved Hide resolved
tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs Outdated Show resolved Hide resolved
Comment on lines 118 to 123
let internal normalizedPath(fieldPath : FieldPath) =
fieldPath |> List.rev

let internal normalizedPathToObj(fieldPath : FieldPath) =
fieldPath |> List.rev |> box

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let internal normalizedPath(fieldPath : FieldPath) =
fieldPath |> List.rev
let internal normalizedPathToObj(fieldPath : FieldPath) =
fieldPath |> List.rev |> box
let internal normalizeErrorPath (fieldPath : FieldPath) =
fieldPath |> List.rev

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I remove normalizedPathToObj function?

src/FSharp.Data.GraphQL.Server/ErrorTypes.fs Outdated Show resolved Hide resolved
src/FSharp.Data.GraphQL.Server/ErrorTypes.fs Outdated Show resolved Hide resolved
@xperiandri xperiandri changed the title Add errors to resolved fields Ability to add errors to resolved fields Feb 5, 2025
@xperiandri xperiandri merged commit 645967f into fsprojects:dev Feb 5, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants