[DEV-IMP] Add error logging and hide sensitive configs from individual jobs#98
Open
moda20 wants to merge 3 commits into
Open
[DEV-IMP] Add error logging and hide sensitive configs from individual jobs#98moda20 wants to merge 3 commits into
moda20 wants to merge 3 commits into
Conversation
…nd-handing [MAIN-Feat] Automated proxy testing system with intelligent selection strategies
Update config shared with jobs to have no sensitive data (explicitly hidden from jobs, encrypted values) small improvement for config categorization for the frontend
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
moda20
changed the title
… when converting schema to job accessible version
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config/config.service.ts`:
- Around line 110-116: The filter using withJobHiddenProperties is ineffective
because propertiesValues[pvk].job_hidden is never set from schema metadata;
update the code that builds propertiesValues (or the schema parsing path) to
populate the job_hidden flag from the schema's metadata (e.g., copy
schemaField.metadata.job_hidden or schemaField.jobHidden into
propertiesValues[pvk].job_hidden) so the conditional in the block referencing
withJobHiddenProperties, propertiesValues, pvk, and job_hidden can correctly
detect and delete job-hidden entries; ensure the property name matches the
schema source and add a unit test for the removal behavior when
withJobHiddenProperties is false.
In `@src/initialization/jobsManager.ts`:
- Line 63: The error log builds a message using JSON.stringify(d, null, 4) which
can throw on circular structures; update the logging in the job start/failure
path (the code that calls job.getName() and uses variable d) to never call
JSON.stringify directly on untrusted objects — wrap serialization in a safe
helper (e.g., try/catch around JSON.stringify with a fallback like util.inspect
or a JSON-safe serializer, or add a safeStringify(d) utility that returns a
non-throwing string), and use that helper when composing the message so the
logger cannot fail due to circular data.
In `@src/jobConsumer/jobConsumer.ts`:
- Around line 247-252: The job-facing options.config currently passes full
flattened property objects via
ObjectifyFlattenedProperties(getConvictSchemaProperties(...)), causing leaves to
be { value, ... } instead of primitives; change the construction so
options.config contains raw values (the previous shape) by extracting each
property's primitive value (e.g., use config.getProperties() or map the output
of getConvictSchemaProperties(...) to its .value for each leaf) before calling
ObjectifyFlattenedProperties or replace ObjectifyFlattenedProperties input with
the flattened values, updating references to ObjectifyFlattenedProperties,
getConvictSchemaProperties, and options.config accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 46e10f03-b16a-400e-bd67-36b05a037db0
📒 Files selected for processing (6)
src/config/config.service.tssrc/config/config.tssrc/initialization/jobsManager.tssrc/jobConsumer/jobConsumer.tssrc/types/models/config.tssrc/utils/convictUtils.ts
Comment on lines
+247
to
+252
| config: ObjectifyFlattenedProperties( | ||
| getConvictSchemaProperties({ | ||
| encryptedValues: false, | ||
| withJobHiddenProperties: false, | ||
| }), | ||
| ), |
Contributor
There was a problem hiding this comment.
Job-facing options.config shape changed from values to metadata objects.
At Line 247, ObjectifyFlattenedProperties receives full flattened property objects, so leaves become { value, ... } instead of raw values (previously from config.getProperties()). This can break existing jobs that read primitives.
Suggested fix
- config: ObjectifyFlattenedProperties(
- getConvictSchemaProperties({
- encryptedValues: false,
- withJobHiddenProperties: false,
- }),
- ),
+ config: ObjectifyFlattenedProperties(
+ Object.fromEntries(
+ Object.entries(
+ getConvictSchemaProperties({
+ encryptedValues: false,
+ withJobHiddenProperties: false,
+ }),
+ ).map(([k, v]) => [k, { value: v.value }]),
+ ),
+ ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/jobConsumer/jobConsumer.ts` around lines 247 - 252, The job-facing
options.config currently passes full flattened property objects via
ObjectifyFlattenedProperties(getConvictSchemaProperties(...)), causing leaves to
be { value, ... } instead of primitives; change the construction so
options.config contains raw values (the previous shape) by extracting each
property's primitive value (e.g., use config.getProperties() or map the output
of getConvictSchemaProperties(...) to its .value for each leaf) before calling
ObjectifyFlattenedProperties or replace ObjectifyFlattenedProperties input with
the flattened values, updating references to ObjectifyFlattenedProperties,
getConvictSchemaProperties, and options.config accordingly.
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
1 participant
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.
Improvements:
Notes
How Has This Been Tested?
Related Issues
Screenshots (if applicable)
Additional Context
Summary by CodeRabbit
New Features
Bug Fixes
Chores