fix: 7 critical bugs in cdm.js and server.js#196
Merged
Conversation
Four calls to the async esJsonArrRequest() were missing await, causing .forEach() to be called on a Promise (silently failing). Two of these also used the obsolete sync-request .getBody() pattern which is incompatible with the async version. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
JSON.stringify(obj.null, 2) accessed obj.null (undefined) and passed 2 as the replacer. Should be JSON.stringify(obj, null, 2) to pretty-print the object with 2-space indentation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
retMag (typo) was assigned instead of retMsg, so the error message was lost and retMsg remained empty on return. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy-paste error: the addIterations block checked addRuns != [] instead of addIterations != [], so addIterations was only processed when addRuns was also non-empty. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The detailed mismatch error (response count vs request count) was immediately overwritten by an unrelated generic error about missing period-id/run-id. Remove the erroneous overwrite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comparing sets[i].breakout != 'undefined' checks against the literal string, not the undefined type. When breakout is actually undefined, the comparison is true (different types), causing .length to be called on undefined (TypeError). Use the existing isDefined() helper which does typeof correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
metric_data was assigned without var/let/const, creating an implicit global variable. Concurrent requests to /api/v1/metric-data could overwrite each other's data, returning wrong results. Add var to make it function-scoped. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@atheurer Can you take a close look at these AI identified bugs and the associated fixes since this is more your area? |
atheurer
approved these changes
Jun 29, 2026
atheurer
left a comment
atheurer
left a comment
Contributor
There was a problem hiding this comment.
All 7 fixes are correct — reviewed each individually:
- JSON.stringify arg order —
obj.null→obj, null, 2✓ - Missing await on esJsonArrRequest — 4 calls were silently getting Promises instead of results ✓
- addRuns → addIterations — wrong variable name ✓
- Error message overwrite — preserved the specific diagnostic ✓
- retMag typo — →
retMsg✓ - String comparison for undefined —
!= 'undefined'→isDefined()✓ - Implicit global metric_data — race condition on concurrent requests ✓
Minor note: fix #3's addIterations != [] is still always-true (JS reference comparison), but that's pre-existing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Seven critical bugs in the CDM query engine, each as a separate commit:
awaiton 4esJsonArrRequest()calls —.forEach()on a Promise silently fails, tag/param filters ingetIters()produce no resultsJSON.stringify(obj.null, 2)wrong arg order — logsundefinedinstead of the object that failed source resolutionretMagtypo — error message assigned to throwaway variable, caller gets emptyret-msgaddRunschecked instead ofaddIterations—--add-iterationsonly works when--add-runsis also non-empty!= 'undefined'string comparison — compares against literal string instead of usingisDefined(), causes TypeError when breakout is actually undefinedmetric_datain server.js — missingvarcreates race condition on concurrent/api/v1/metric-datarequestsCloses #194
Test plan
node --checkpasses on both files🤖 Generated with Claude Code