Enhance logging for parameter query results in BucketChecksumS#462
Enhance logging for parameter query results in BucketChecksumS#462hamzzy wants to merge 2 commits intopowersync-ja:mainfrom
Conversation
🦋 Changeset detectedLatest commit: f1cdb21 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
rkistner
left a comment
rkistner
left a comment
There was a problem hiding this comment.
Thanks for the PR! This will help a lot with debugging.
I added some comments, mostly just around creating separate functions to avoid bloating the existing functions too much.
There are also some minor updates to the tests required in the meantime due to refactoring on the main branch.
| message += `buckets: ${allBuckets.length}`; | ||
| if (parameterQueryResultsByDefinition) { | ||
| const totalParamResults = Array.from(parameterQueryResultsByDefinition.values()).reduce( | ||
| (sum, count) => sum + count, | ||
| 0 | ||
| ); | ||
| message += ` | param_results: ${totalParamResults}`; | ||
| } | ||
| message += ` ${limitedBuckets(allBuckets, 20)}`; | ||
| const logData: any = { checkpoint: base.checkpoint, user_id: user_id, buckets: allBuckets.length }; | ||
| if (parameterQueryResultsByDefinition) { | ||
| const totalParamResults = Array.from(parameterQueryResultsByDefinition.values()).reduce( | ||
| (sum, count) => sum + count, | ||
| 0 | ||
| ); | ||
| logData.parameter_query_results = totalParamResults; | ||
| } | ||
| this.logger.info(message, logData); |
There was a problem hiding this comment.
The log message code is getting quite long here - can this be moved to a separate function?
| this.logger.error(error.message, { | ||
|
|
||
| // Build breakdown of parameter query results by definition | ||
| let errorMessage = error.message; |
There was a problem hiding this comment.
Same here - can we move the code to generate this message to a separate function?
| if (update.parameterQueryResultsByDefinition.size > 10) { | ||
| errorMessage += `\n ... and ${update.parameterQueryResultsByDefinition.size - 10} more`; | ||
| } |
There was a problem hiding this comment.
For this, can we log the remaining count, instead of or in addition to the number of definitions?
2 participants
Summary
Adds detailed logging for parameter query results to improve debugging when limits are exceeded.
Problem
When parameter query results exceed the configured limit, users only see a generic error message:
[PSYNC_S2305] Too many parameter query results: 1234 (limit of 1000)
This makes it difficult to debug because:
Solution
1. Enhanced Normal Logging
Checkpoint logs now include total parameter query results:
New checkpoint: 1 | write: null | buckets: 2 | param_results: 2
Updated checkpoint: 2 | write: null | buckets: 3 | param_results: 3
2. Detailed Error Breakdown
Error messages now show the top 10 sync stream definitions by result count:
[PSYNC_S2305] Too many parameter query results: 60 (limit of 50)
Parameter query results by definition:
projects: 30
tasks: 20
comments: 10
Implementation Details
getCheckpointUpdateDynamic()Testing
Added 3 comprehensive test cases:
All tests passing: 17/17 in
BucketChecksumState.test.tsChecklist