Improving server exception logging by including exception in log message #471
Conversation
This file contains 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
…n info too
There were two problems before:
1. The error message itself was not being logged, but only a mention
that there was an error;
2. It was not possible to access the possibly original exception
which led to the request processing error. However, it's
extremely useful to log the error with the whole exception
information including the stack trace of where the exception was thrown.
However, these changes also introduce a breaking change as IGQLError
and GQLProblemDetails now each have an additional mandatory member
holding the possible exception which generated the error.
These types (especiall GQLProblemDetails) are public and could be used
by code using FSharp.Data.GraphQL.
xperiandri
reviewed
Apr 6, 2024
This reverts commit 46d5b76.
This reverts commit d24abb4.
… active pattern" This reverts commit b03bf40.
…exception info too" This reverts commit e829181.
…info too
Note: this commit is a 2nd attempt after getting feedback from code
review.
There were two problems before:
1. The error message itself was not being logged, but only a mention
that there was an error;
2. It was not possible to access the possibly original exception
which led to the request processing error. However, it's
extremely useful to log the error with the whole exception
information including the stack trace of where the exception was thrown.
However, these changes also introduce a breaking change as IGQLError
and GQLProblemDetails now each have an additional mandatory member
holding the possible exception which generated the error.
These types (especiall GQLProblemDetails) are public and could be used
by code using FSharp.Data.GraphQL.
|
@xperiandri I rewrote my changes according to your suggestion regarding the extra interface. Please kindly review the current changes. |
xperiandri
reviewed
Apr 10, 2024
src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs
Outdated
Show resolved
Hide resolved
These changes are according to a request during code review.
|
@xperiandri there's a commit for applying fantomas formatting to |
xperiandri
reviewed
Apr 10, 2024
There was a problem hiding this comment.
@valbers it is something bad on your side. I've just formatted it on my side and it is similar to what it was before your formatting.
Try restore the latest Fantomas with tool restore in this repo
|
@xperiandri waiting for your approving review. |
xperiandri
reviewed
Apr 12, 2024
src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrii Chebukin <XperiAndri@Outlook.com>
xperiandri
approved these changes
Apr 12, 2024
|
Cool! Lots of thanks for the contribution! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
There were two problems before:
Before
After
However, these changes also introduce a breaking change as
GQLProblemDetailsnow has an additional member holding the possible exception which generated the error.GQLProblemDetailsis public and could be in use by code using FSharp.Data.GraphQL.