Skip to content

Pass a res.locals object to the dbs connection from the query - #2973

Open
AlexanderGeere wants to merge 6 commits into
GEOLYTIX:patchfrom
AlexanderGeere:eng-386-query-stitch
Open

AlexanderGeere wants to merge 6 commits into
GEOLYTIX:patchfrom
AlexanderGeere:eng-386-query-stitch

Conversation

@AlexanderGeere

@AlexanderGeere AlexanderGeere commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Description

Pass res.locals to the dbs connection as options.locals

The query module now passes res.locals to the dbs connection as
options.locals:
{ locals } for a blocking query
{ locals, nonblocking: true } for a nonblocking one.

The dbs module only reads options.nonblocking, so options.locals is ignored and nothing changes for the connections it creates.

res.locals would be an object containing what the hosts db connection wants e.g { dbs: {tenant_id: 7} }

Why

A composing host can replace a connection in the dbs module's exported object with its own function e.g. to run every query on that connection inside a transaction with settings taken from the request.

Its middleware puts the values that connection needs on res.locals. Only that object is passed, not the whole request. Clients can't write to res.locals, unlike req.params, which the query string is merged into.

Tests check that the context reaches the connection for both blocking and nonblocking queries, and that the request isn't passed.

GitHub Issue

Type of Change

  • ✅ New feature (non-breaking change which adds functionality)
  • ✅ Documentation
  • ✅ Testing

How have you tested this?

Ran the tests written here

Testing Checklist

  • ✅ Existing Tests still pass
  • ✅ New Tests Added
  • ✅ Ran locally on my machine

Code Quality Checklist

  • ✅ My code follows the guidelines of XYZ
  • ✅ My code has been commented
  • ✅ Documentation has been updated
  • ✅ New and existing unit tests pass locally with my changes
  • ✅ Main has been merged into this PR

Summary by CodeRabbit

  • Refactor
    • Standardized how database queries receive their settings across regular, nonblocking, and logging-related operations. Query execution and response behavior remain unchanged, including the existing response for nonblocking requests.
  • Tests
    • Updated query, database utility, and logging tests to verify the standardized settings and confirm existing query behavior remains intact.

…module.

The dbs module ignores options.req. A composing host which replaces a dbs connection can read the request context from it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5e624739-cb00-4bf3-b26d-bcd716bcea89
📥 Commits

Reviewing files that changed from the base of the PR and between f37bff4 and 3b97905.

📒 Files selected for processing (6)
  • apps/xyz/mod/query.js
  • apps/xyz/mod/utils/dbs.js
  • apps/xyz/mod/utils/logger.js
  • apps/xyz/tests/mod/query.test.mjs
  • apps/xyz/tests/mod/utils/dbs.test.mjs
  • apps/xyz/tests/mod/utils/logger.test.mjs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Database query calls now pass query settings through one params object instead of positional arguments. The query module includes res.locals in that object. The database utility and PostgreSQL logger also use the params-object form.

Changes

Database query params

Layer / File(s) Summary
Pass query context to database connections
apps/xyz/mod/query.js, apps/xyz/tests/mod/query.test.mjs
Blocking and nonblocking calls pass query settings in one params object. The query module includes res.locals; tests check the object fields and confirm that it does not include the request object.
Accept query params in clientQuery
apps/xyz/mod/utils/dbs.js, apps/xyz/tests/mod/utils/dbs.test.mjs
clientQuery accepts query, variables, timeout, and nonblocking settings in one object. Tests update blocking, nonblocking, timeout, retry, and logging calls to use that object.
Pass logger queries through params objects
apps/xyz/mod/utils/logger.js, apps/xyz/tests/mod/utils/logger.test.mjs
The PostgreSQL logger passes query, variables, timeout, and nonblocking settings in one object. Its test checks those fields.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 3b979

This change moves database query settings into one params object and passes res.locals to the connection. The call sites and the consumer appear consistent, and no actionable merge-blocking risk remains.

Architecture Summary

Architecture risk: 🟡 Medium · up to 3b979

The change affects 1 system.

Changed systems: apps/xyz

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/xyz (service) was modified; 6 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/xyz/mod/query.js: The query documentation now describes the connection params object and res.locals handoff to host-replaced connections, and states that clients cannot write to res.locals.
  • observed — Modified behavior in apps/xyz/mod/query.js: The nonblocking path now passes a shared params object containing locals, nonblocking, query, timeout, and variables, replacing positional arguments and a separate nonblocking options object.
  • observed — Modified behavior in apps/xyz/mod/query.js: The regular query path now passes the same params object instead of positional query, SQL-variable, and timeout arguments.
  • observed — Modified behavior in apps/xyz/mod/utils/dbs.js: clientQuery now accepts a params object instead of separate query arguments and an options object. Its documentation places nonblocking at the top level and describes extra properties as ignored by this method but readable by a host-replaced connection method. The implementation reads query and variables from params, defaults timeout from params.timeout to xyzEnv.STATEMENT_TIMEOUT, and selects blocking execution using params.nonblocking.

Reliability and maintainability

  • inferred — Risk-relevant change factors for apps/xyz: blast_radius_1; direct_dependents_1
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: passing res.locals to the dbs connection from the query. It is concise and specific.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 6 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

@AlexanderGeere AlexanderGeere changed the title Pass the request to the dbs connection as options.req from the query … Pass the request to the dbs connection as options.req from the query Oct 2, 2026
Only the values a host sets for its dbs connections are passed, not the whole request. Clients cannot write to res.locals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AlexanderGeere AlexanderGeere changed the title Pass the request to the dbs connection as options.req from the query Pass a context object to the dbs connection as options.req from the query Oct 2, 2026
@AlexanderGeere
AlexanderGeere marked this pull request as ready for review October 2, 2026 09:28
@AlexanderGeere AlexanderGeere self-assigned this Oct 2, 2026
@AlexanderGeere AlexanderGeere added Feature New feature requests or changes to the behaviour or look of existing application features. Testing Changes relating to existing or new unit tests. Documentation Adding to or revising existing documentation. labels Oct 2, 2026
@AlexanderGeere AlexanderGeere changed the title Pass a context object to the dbs connection as options.req from the query Pass a res.locals object to the dbs connection as options.req from the query Oct 2, 2026
@dbauszus-glx

Copy link
Copy Markdown
Member

I asked Claude to consolidate the individual params into a params object.

This disolves the options param into the params object.

The benefits are:
We no longer need a set order eg. query, variables, timeout, options.
We want to have the option to support different data stores in the future which may require a different set of params.

@dbauszus-glx

Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@dbauszus-glx

Copy link
Copy Markdown
Member

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@AlexanderGeere

AlexanderGeere commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

@dbauszus-glx

I asked Claude to consolidate the individual params into a params object.

This disolves the options param into the params object.

The benefits are: We no longer need a set order eg. query, variables, timeout, options. We want to have the option to support different data stores in the future which may require a different set of params.

This makes sense to me, much more expandable going forward.
Works for our use case as well.

@AlexanderGeere AlexanderGeere changed the title Pass a res.locals object to the dbs connection as options.req from the query Pass a res.locals object to the dbs connection from the query Oct 2, 2026
@RobAndrewHurst

Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@AlexanderGeere

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation Adding to or revising existing documentation. Feature New feature requests or changes to the behaviour or look of existing application features. Testing Changes relating to existing or new unit tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants