[ENG-4117] fix: adds better auth logs #70
Conversation
gupta-piyush19
changed the title
Greptile OverviewGreptile SummaryThis PR improves authentication error logging by providing clearer guidance when users encounter 401/403 errors, helping them identify when they're using the wrong Infisical instance (US vs EU vs self-hosted). Key Changes:
Issues Found:
Confidence Score: 5/5
Important Files ChangedFile Analysis
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func extractDomainHint(urlStr string) string { | ||
| if urlStr == "" { | ||
| return "" | ||
| } | ||
|
|
||
| if parsedURL, err := url.Parse(urlStr); err == nil && parsedURL.Host != "" { | ||
| return parsedURL.Scheme + "://" + parsedURL.Host | ||
| } | ||
|
|
||
| return "" | ||
| } |
There was a problem hiding this comment.
style: This function is duplicated in packages/util/log.go:225-235. Consider consolidating into a shared utility function (e.g., in packages/util/helper.go) to maintain DRY principles.
There was a problem hiding this comment.
getting dependency cycle in moving this function to util
There was a problem hiding this comment.
I understand the dependency cycle issue. The packages/api package likely imports packages/util, so moving a function from api to util would create a circular dependency when api tries to use it.
Here are a few solutions to resolve this:
Option 1: Keep the duplication for now
Since this is a simple utility function and the dependency cycle prevents consolidation, it might be acceptable to keep both versions until a larger refactoring can address the circular dependencies.
Option 2: Create a new shared package
Create a new package like packages/common or packages/internal/urlutil that both api and util can import without creating cycles.
Option 3: Move the function to a more fundamental package
If there's a package that's lower in the dependency hierarchy than both api and util, you could move it there.
Option 4: Refactor to break the cycle
Review why api imports util and see if some of that functionality could be moved or restructured to break the circular dependency.
For now, I'd recommend Option 1 - keeping the duplication temporarily and adding a TODO comment to track the technical debt:
| func extractDomainHint(urlStr string) string { | |
| if urlStr == "" { | |
| return "" | |
| } | |
| if parsedURL, err := url.Parse(urlStr); err == nil && parsedURL.Host != "" { | |
| return parsedURL.Scheme + "://" + parsedURL.Host | |
| } | |
| return "" | |
| } | |
| // TODO: This function is duplicated in packages/util/log.go:225-235 | |
| // Consider consolidating when circular dependency between api and util packages is resolved | |
| func extractDomainHint(urlStr string) string { | |
| if urlStr == "" { | |
| return "" | |
| } | |
| if parsedURL, err := url.Parse(urlStr); err == nil && parsedURL.Host != "" { | |
| return parsedURL.Scheme + "://" + parsedURL.Host | |
| } | |
| return "" | |
| } |
Which approach would you prefer to take?
Description 📣
adds better auth logs
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets