Logging errors emitted for find requests with fields selection not including required fields

[onhate]

I've seen many changes being pushed directly to main and it makes it harder to track the changes.

Right now I have an issue where the lib is logging errors for required fields on count queries which is non-sense and I cannot figure out from where that change came

the first commit I tracked is this one https://github.com/sensedeep/dynamodb-onetable/commit/e20250a6eaf27e4441e25dfdc9797304ed728efe but the message was already there from another commit.

I need help figuring this out.

Also, CI is failing for all pushes directly to main what makes my confidence on the lib quality decrease.

[mobsense]

Can you please give more details:

What kind of count request? What was the error?

Can you give a test case that demonstrates the issue.

Regarding PRs, yes that would be ideal to have a 100% PR based dev cycle. We'll try to at least better stage the commits.

There was a two recent changes:

1. In 2.7.3 a fix was made to detect if value template values that were required in unique fields, were missing.
2. In 2.7.4 a fix was made to set undefined fields that now have a default value in the schema, to the default values on get/find queries.

There was one other change for value templates to fix quotes around the terms in "where" clauses.

Please submit a short debug.ts that shows the problem and request.

[mobsense]

I'm just guessing, but this proposed change may fix your issue.

As part of the setting undefined fields to default values, the warning highlighted may be emitting the error you see. Previously, the code above silently ignored undefined required fields and did not set them to default values. Now, the code loop you highlighted is being executed.

The patch below will only emit the warning to the log file if:

• Not a transaction and
• params.count not set and params.select not set to 'COUNT'
• params.fields not set to retrieve a subset of fields

If you can, please test this patch and if it looks good for you, let us know asap and we'll see if we can roll out an update. It passes unit tests and we are testing this on our production systems now.

diff --git a/src/Model.js b/src/Model.js
index a2d459a..be4c3c0 100644
--- a/src/Model.js
+++ b/src/Model.js
@@ -1136,11 +1136,11 @@ export class Model {
                 }
             } else if (value === undefined) {
                 if (field.required) {
                    /*
                         Transactions transform the properties to return something, but
                         does not have all the properties and required fields may be missing)
                      */
-                    if (!params.transaction) {
+                    if (!params.transaction && !params.count && params.select != 'COUNT' && !params.fields) {
                         this.table.log.error(`Required field "${name}" in model "${this.name}" not defined in table item`, {
                             model: this.name, raw, params, field,
                         })

[onhate]

ok, after some debugging I've got it. It happens on find with fields selection.

In this case race and breed are required but I did not select them on the find operation, and because of that it logs an error.

Pet: {
            pk: {type: String, value: '${_type}', hidden: true},
            sk: {type: String, value: '${_type}#${id}', hidden: true},
            id: {type: String, generate: 'ulid'},
            name: {type: String},
            race: {type: String, enum: ['dog', 'cat', 'fish'], required: true},
            breed: {type: String, required: true},
        }

[onhate]

I've raised this PR with a test that reproduces the issue

https://github.com/sensedeep/dynamodb-onetable/pull/533

[onhate]

why reopen?

[mobsense]

Just so that people who saw the issue can more easily see the resolution for a week.

[mobsense]

Thanks for the quick PR

[martinmicunda]

@mobsense there is same problem with batchGet operation. If I add && !params.batch it solve the issue.

if (!params.transaction && !params.fields && !params.batch) {
   this.table.log.error(`Required field "${name}" in model "${this.name}" not defined in table item`, {
     model: this.name, raw, params, field,
   });
}

[mobsense]

Thank you.

The line now looks like:

if (!params.transaction && !params.batch && !params.fields && !field.encode && !params.noerror)

We're adding params.noerror to turn off if required.