Skip to content
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

feat: make error messages more clear #1166

Merged
merged 1 commit into from
Sep 14, 2023
Merged

feat: make error messages more clear #1166

merged 1 commit into from
Sep 14, 2023

Conversation

mamadoudicko
Copy link
Contributor

Make axios error messages more clear: #1123

@mamadoudicko mamadoudicko temporarily deployed to preview September 13, 2023 15:14 — with GitHub Actions Inactive
@vercel
Copy link

vercel bot commented Sep 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 13, 2023 3:18pm
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 13, 2023 3:18pm

@github-actions
Copy link
Contributor

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/app/chat/[chatId]/components/ActionsBar/hooks/useKnowledgeUploader.ts

The changes in this file are mostly about error handling. The developer has added a helper function getAxiosErrorParams to extract error parameters from the Axios error object. This is a good practice as it makes the error handling code cleaner and more maintainable. However, the developer should consider adding type checking for the error parameter in the getAxiosErrorParams function to ensure it is an instance of AxiosError. This can be done using the instanceof operator. For example:

if (error instanceof AxiosError) {
  // Extract error parameters
}

This will prevent potential runtime errors if the function is called with an argument that is not an instance of AxiosError.


Risk Level 3 - /home/runner/work/quivr/quivr/frontend/app/chat/[chatId]/components/ChatDialogueArea/components/ChatDialogue/components/ChatNotification/components/NotificationDisplayer/NotificationDisplayer.tsx

The developer has added a console.log statement which is generally not recommended for production code as it can expose sensitive information and clutter the console output. It's better to remove this or replace it with a proper logging library that can be configured to log only in development mode. For example:

if (process.env.NODE_ENV === 'development') {
  console.log({
    message,
    status,
    name,
  });
}

This will ensure that the log statement is only executed in development mode.


🔍🐛🔧


Powered by Code Review GPT

@gozineb gozineb merged commit 686612a into main Sep 14, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants