fix: log fetch failure#532
Conversation
📝 WalkthroughWalkthroughIn ChangesSession Fetch Error Logging
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/app/composables/session.ts`:
- Line 35: The arrow function parameter in the `.catch()` method violates the
`@stylistic/arrow-parens` linting rule. Wrap the single parameter `reason` in
parentheses by changing `.catch(reason => {` to `.catch((reason) => {` to comply
with the linting requirement.
- Around line 35-37: The catch handler in the session composable is logging the
raw rejection reason object directly, which can expose sensitive authentication
and session details in logs. Replace the direct logging of the raw `reason`
parameter with a sanitized error message that does not include sensitive
information. If error details are needed for development debugging, consider
logging only safe, minimal information and only when in development mode, while
production should use a generic safe message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7aa40195-9031-47cf-b88d-9a10c947b1d6
📒 Files selected for processing (1)
src/runtime/app/composables/session.ts
| }, | ||
| retry: false, | ||
| }).catch(() => null) | ||
| }).catch(reason => { |
There was a problem hiding this comment.
Fix lint error for arrow function parentheses.
Line 35 currently violates @stylistic/arrow-parens; this can fail lint/CI.
Minimal lint fix
- }).catch(reason => {
+ }).catch((reason) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }).catch(reason => { | |
| }).catch((reason) => { |
🧰 Tools
🪛 ESLint
[error] 35-35: Expected parentheses around arrow function argument having a body with curly braces.
(@stylistic/arrow-parens)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/app/composables/session.ts` at line 35, The arrow function
parameter in the `.catch()` method violates the `@stylistic/arrow-parens`
linting rule. Wrap the single parameter `reason` in parentheses by changing
`.catch(reason => {` to `.catch((reason) => {` to comply with the linting
requirement.
Source: Linters/SAST tools
| }).catch(reason => { | ||
| console.log('Failure on fetching session', reason) | ||
| return null |
There was a problem hiding this comment.
Avoid logging raw fetch rejection objects.
At Line 36, logging reason directly can leak sensitive auth/session error details into client/server logs. Prefer a sanitized message (and only include minimal safe fields in development mode).
Suggested safer pattern
- }).catch(reason => {
- console.log('Failure on fetching session', reason)
+ }).catch((reason) => {
+ const message = reason instanceof Error ? reason.message : String(reason)
+ if (import.meta.dev) {
+ console.error('Failure on fetching session:', message)
+ }
return null
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }).catch(reason => { | |
| console.log('Failure on fetching session', reason) | |
| return null | |
| }).catch((reason) => { | |
| const message = reason instanceof Error ? reason.message : String(reason) | |
| if (import.meta.dev) { | |
| console.error('Failure on fetching session:', message) | |
| } | |
| return null | |
| }) |
🧰 Tools
🪛 ESLint
[error] 35-35: Expected parentheses around arrow function argument having a body with curly braces.
(@stylistic/arrow-parens)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/app/composables/session.ts` around lines 35 - 37, The catch
handler in the session composable is logging the raw rejection reason object
directly, which can expose sensitive authentication and session details in logs.
Replace the direct logging of the raw `reason` parameter with a sanitized error
message that does not include sensitive information. If error details are needed
for development debugging, consider logging only safe, minimal information and
only when in development mode, while production should use a generic safe
message.
| }).catch(() => null) | ||
| }).catch(reason => { | ||
| console.log('Failure on fetching session', reason) | ||
| return null |
There was a problem hiding this comment.
We may somewhat fix the issue by yielding sessionState.value here instead of null. It would allow to preserve the session state on fetch failure, instead of losing it.
1 participant
Help diagnose #488
Summary by CodeRabbit