[1808] User Router Logging Fix#1809
Merged
megatnt1122 merged 9 commits intodevelfrom Jan 14, 2026
Merged
Conversation
Contributor
Reviewer's GuideAdjusts user_router request logging to remove meaningless 'extra: "undefined"' payloads and enrich several routes with more informative context, particularly for user lookup, deletion, key management, token access, and identity listing/removal. Sequence diagram for updated /ident/list logging with enriched extra datasequenceDiagram
actor User
participant UserRouter
participant g_lib
participant g_db
participant logger
User->>UserRouter: GET /ident/list?client=...
UserRouter->>g_lib: getUserFromClientID(client)
g_lib-->>UserRouter: client
UserRouter->>g_db: query ident edges for client
g_db-->>UserRouter: extra_log (list of ident keys)
UserRouter->>logger: logRequestSuccess(client, correlationId, GET, /ident/list, Success, description, extra = { NumOfIds: extra_log })
logger-->>UserRouter: ack
UserRouter-->>User: 200 OK with ident list
rect rgb(240,240,240)
User->>UserRouter: GET /ident/list (error case)
UserRouter->>g_lib: getUserFromClientID(client)
g_lib-->>UserRouter: client or error
UserRouter->>g_db: query ident edges for client (may fail)
g_db-->>UserRouter: extra_log or error
UserRouter->>logger: logRequestFailure(client, correlationId, GET, /ident/list, Failure, description, extra = { NumOfIds: extra_log }, error)
logger-->>UserRouter: ack
UserRouter-->>User: Error response
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
megatnt1122
added
Type: Refactor
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In the
/ident/listhandler,extra_logis set to the raw query result and then logged asNumOfIds; consider converting this to a count or an array of keys so the logged structure is explicit and doesn’t depend on the database cursor implementation. - The additional context added to log descriptions (e.g.
name_uid,user_id,ident) may include user identifiers; verify that logging these values aligns with your PII/sensitive-data logging policies, or mask/shorten them if necessary.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `/ident/list` handler, `extra_log` is set to the raw query result and then logged as `NumOfIds`; consider converting this to a count or an array of keys so the logged structure is explicit and doesn’t depend on the database cursor implementation.
- The additional context added to log descriptions (e.g. `name_uid`, `user_id`, `ident`) may include user identifiers; verify that logging these values aligns with your PII/sensitive-data logging policies, or mask/shorten them if necessary.
## Individual Comments
### Comment 1
<location> `core/database/foxx/api/user_router.js:1544-1545` </location>
<code_context>
- description: "Remove existing user entry",
- extra: user_id,
+ description: `Remove existing user entry: ${user_id}`,
+ extra: {
+ subject: req.query?.subject ?? null,
+ },
});
</code_context>
<issue_to_address>
**issue (bug_risk):** `req.query?.subject` is inconsistent with `req.queryParams` usage and may always be `undefined`.
Elsewhere in this file query params are accessed via `req.queryParams`, so `req.query?.subject` will likely always be `undefined` and `subject` will never be logged. To correctly capture the query parameter and stay consistent, use `req.queryParams.subject ?? null` instead.
</issue_to_address>
### Comment 2
<location> `core/database/foxx/api/user_router.js:1611-1608` </location>
<code_context>
client: client._id,
}),
);
+ extra_log = g_db._query("for v in 1..1 outbound @client ident return v._key", {
+ client: client._id,
+ });
logger.logRequestSuccess({
</code_context>
<issue_to_address>
**issue (bug_risk):** `extra_log` stores a cursor but is logged as `NumOfIds`, which is misleading and likely not what was intended.
Since `g_db._query` returns a cursor, this will log the cursor object rather than a count. If you want to record how many linked IDs there are, derive a numeric value first (e.g., `const numOfIds = extra_log.count()` or by materializing the results and using `.length`) and log that instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Collaborator
|
So this also needs to be fixed in the user_router. 
There may be quite a lot of endpoints 
You need to edit this too, maybe return the most recent one and also the number of endpoints found |
Collaborator
|
Can you add the subject (not client) in the description in the user token set commands. 
Subject should revert to client if subject is not defined. This should actually be true for every route that has a subject. |
Collaborator
|
Password success auth is printing undefined, making us think the logs are not set up correct. 
|
t-ramz
reviewed
Dec 17, 2025
Collaborator
t-ramz
left a comment
t-ramz
left a comment
There was a problem hiding this comment.
So a couple thoughts/questions:
- What does this fix, exactly? Just to fill me in, I'm behind. From what I'm seeing the order is being fixed so that the logger at least gets a message regardless of any kind of success.
- Are there any places where the string interpolation can bite us? I see we're trusting params, which could be fine depending on use. I just don't want this to go overlooked. e.g.
let sub = req.queryParams.subject ? req.queryParams.subject : req.queryParams.client;
...
description: `Set user public and private keys. Subject: ${sub}`,
t-ramz
approved these changes
Dec 18, 2025
Collaborator
t-ramz
left a comment
t-ramz
left a comment
There was a problem hiding this comment.
Looks good!
Changes were for consistent naming on clients and to avoid confusion in "undefined" vs "not applicable"
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
3 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.
Ticket
#1808
Description
Fixes for user_router's logging
Tasks
Summary by Sourcery
Improve user router request logging for authentication, key, token, and identity management endpoints.
Enhancements:
extra: "undefined"fields from multiple user router log entries.