Add context to fs_napi errors #8149
Merged
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
guymguym
requested changes
Jun 26, 2024
tangledbytes
force-pushed
the
utkarsh/fix/8065
branch
2 times, most recently
from
edbc059 to
a25982d
Compare
guymguym
approved these changes
Jul 7, 2024
Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>
tangledbytes
force-pushed
the
utkarsh/fix/8065
branch
from
a25982d to
5d8dda6
Compare
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.
Explain the changes
This PR attempts to add
contextto the errors coming fromfs_napi. Thecontextfield is then used in the S3 error reporting if it's available. The PR also reduces the log level forOnErrormethod.This sometimes means duplicate logging, however it was still kept this way as not all the errors coming from the
fs_napinecessarily fails an S3 operation. It could be helpful to see all the errors that we encountered and the error that resulted in the operation failure.Another approach that was tested but I decided against it was to add the context info in the error message itself. It removes the construct of
contextbut that also means that the error message will be more verbose and hard/fragile===tests against them.Example of current fix:
Issues: Fixed #xxx / Gap #xxx
Fixes #8065