-
Notifications
You must be signed in to change notification settings - Fork 902
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: sync endpoint error handling #2132
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
Thank you for following the naming conventions for pull request titles! 🙏 |
packages/js/src/lib/initialize.tsConsider using a more descriptive variable name than 'e' for the error object in the catch block. This will improve the readability of your code and make it easier for others to understand. try {
existingConfig = config.get();
} catch (error) {
logger.debug("No existing configuration found.");
}
|
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.
@pandeymangg Thank you for the PR 😊💪 I changed a few things up:
- When formbricks-js enters the error state it gets deinitialized. This makes our status management easier as formbricks-js can't be initialized and in an error state
- The error state management doesn't happen in the sync but in the functions that use the sync. that makes it easier to handle the different cases and we don't bloat the sync function with different checks.
- As we currently have some users sending a lot of attributes, I also removed the sync after changing an attribute.
What does this PR do?
Adds a new
status
field to theTJsConfig
type, which indicates that the formbricks js sdk is in an error state. Its set after a sync call fails and then theinitialize
function checks for the error state to skip initializationFixes # (issue)
How should this be tested?
Checklist
Required
pnpm build
console.logs
git pull origin main
Appreciated