-
Notifications
You must be signed in to change notification settings - Fork 727
Fix issue with initial project query request failing #2730
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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 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.
💡 Codebase verification
🛠️ Refactor suggestion
Consider consistent error handling across all tenant-dependent methods
The null check for
tenantIdis currently only implemented inqueryProjectGroups(), but all other methods in this service also depend on a valid tenant ID. Similar protection should be added to all methods to prevent potential API errors.Here are the methods that need the same protection:
findSegment()listSegmentsByIds()updateSegment()createProjectGroup()queryProjects()createProject()createSubProject()getUser()fetchAuditLogs()The original suggestion to extract this check into a utility function is valid and would help maintain consistency across all these methods.
🔗 Analysis chain
Consider consistent error handling across all tenant-dependent methods
While this defensive check prevents the initial project query failure, similar protection might be needed in other methods that depend on tenant ID.
Consider extracting this check into a utility function and applying it consistently:
Let's check other methods that might need similar protection:
The previous search revealed many usages of
getTenantId()across the codebase, but we need to focus specifically on thelf-segments-service.jsfile to see which methods need similar protection. Let's analyze the methods in this file to make a more targeted recommendation.Let's try using ripgrep instead to analyze the methods in lf-segments-service.js that use getTenantId() and see their surrounding context to determine which ones need similar null checks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 66080
Script:
Length of output: 307
Script:
Length of output: 2238