Sync token #239
Sync token #239
Conversation
Staging -->. master
abhishek305
left a comment
abhishek305
left a comment
There was a problem hiding this comment.
- I would suggest just revisit the sync retry logic instead of refactoring the existing flow
| exports.setGlobalConfig = (config) => { | ||
| globalConfig = { ...config }; // Creates a new copy to avoid unintended mutations | ||
| }; |
There was a problem hiding this comment.
what is globalConfig ?
There was a problem hiding this comment.
globalConfig is just a way to store the config safely so we don’t accidentally modify the original object. It helps keep things consistent across different functions that need the same config values (like httpRetries for retries).
To prevent accidental overwrites, we use setGlobalConfig() to update it safely.
It was added to make sure we always use the correct config throughout the module.
Let me know if you think it could cause any issues.. @abhishek305
src/fetch.js
Outdated
| activity.end(); | ||
|
|
||
| return { contentstackData }; | ||
| return { contentstackData: syncData.data } |
There was a problem hiding this comment.
would this create an issue when accessing syncData as previously it was contentstackData.syncData ? now its changed?
There was a problem hiding this comment.
Great Catch, let me remove that possibility by restoring the data structure..
| const normalizeLimit = (limit) => { | ||
| if (limit > 100) { | ||
| console.error('Limit cannot exceed 100. Setting limit to 50.'); | ||
| return 50; | ||
| } | ||
| return limit; | ||
| }; |
There was a problem hiding this comment.
what is this limit ? and do we actually need to refactor that?
There was a problem hiding this comment.
That is a limitation that limit value cannot be set more than 100, incase user does that, we should by default send 50 to the api.
Hence normalizing the logic, incase we make any changes to this limitation, it will be easy to find and make any changes needed
|
@Contentstack-AnkitaD lets close this PR and refactor the retry approach for synctoken |
Refactor the fetch.js file inorder to make debugging the sync token error more better and easy to understand.