Skip to content

Conversation

@bbrks
Copy link
Member

@bbrks bbrks commented Dec 24, 2025

CBG-4413

Captures log output from sync function execution to be piped back into the diagnostic API response

  • Also fixes regression in SyncRunner pool use from Diagnostic API
  • Refactor tests for clearer test cases and improved reliability

Example:

function(doc){
	console.log("this is my doc: "+JSON.stringify(doc));
	console.error("oops!");
	channel(doc.channels);
}
$ curl http://localhost:4987/db1/_sync -H 'Content-Type: application/json' -d '{"doc":{"foo":"bar","channels":["asdf"]}}' | jq
{
  "channels": [
    "asdf"
  ],
  "access": {},
  "roles": {},
  "logging": {
    "errors": [
      "oops!"
    ],
    "info": [
      "this is my doc: {\"_id\":\"\",\"_rev\":\"1-4f209ea9d3dd051b91697a7d5dbf78a6\",\"channels\":[\"asdf\"],\"foo\":\"bar\"}"
    ]
  }
}

Integration Tests

- Also fixes regression in SyncRunner pool use from Diagnostic API
- Refactor tests for clearer test cases and improved reliability
@bbrks bbrks requested a review from RIT3shSapata December 24, 2025 18:12
Copilot AI review requested due to automatic review settings December 24, 2025 18:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds logging capture functionality to the Sync Function diagnostic API, allowing console.log and console.error output from sync function execution to be returned in the API response. The changes also fix a regression related to SyncRunner pool usage and refactor tests for improved clarity and reliability.

Key changes:

  • Added logging capture via callback functions to sync function dry runs
  • Refactored sync function dry run tests for better test case organization
  • Fixed SyncRunner instantiation to create new instances for dry runs instead of using the database's pooled runner

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
rest/diagnostic_doc_api.go Added logging capture fields and callback functions to collect console output during sync function execution
rest/diagnostic_doc_api_test.go Refactored test cases to use clearer naming and structure, added tests for logging capture functionality
db/crud.go Modified SyncFnDryrun to create dedicated sync runner instances with logging callbacks instead of reusing database's channel mapper
channels/sync_runner.go Added NewSyncRunnerWithLogging function to support custom logging callbacks and refactored NewSyncRunner to use it

RequireStatus(t, rt.SendDiagnosticRequest(http.MethodPost, "/{{.keyspace}}/_sync", `{"doc": {"invalid_json"}`), http.StatusBadRequest)
// invalid doc body type
RequireStatus(t, rt.SendDiagnosticRequest(http.MethodPost, "/{{.keyspace}}/_sync", `{"doc": "invalid_doc"}`), http.StatusBadRequest)
// invalid doc body type
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 1363 is duplicated from line 1362. The comment should clarify that line 1364 tests for a forbidden field in the document body, not an invalid type.

Suggested change
// invalid doc body type
// forbidden field in doc body

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@RIT3shSapata RIT3shSapata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@RIT3shSapata RIT3shSapata merged commit ea954fd into main Dec 29, 2025
42 checks passed
@RIT3shSapata RIT3shSapata deleted the CBG-4413 branch December 29, 2025 06:01
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.

3 participants