New "Required field not defined" error is thrown when getting by keys only indexes

[AlexStansfield]

Noticed the size of my logs went up about about 50x (no joke).

We have some operations that get thousands of records by an index that is keys only. For every record it spits out an error for every missing required field. For example one request that gets only 200 records, created almost 1000 error messages.

Schema For the Index:

    [ETechIndexes.GET_BY_BATCH]: {
      hash: 'getByBatchPk',
      sort: 'getByBatchSk',
      project: 'keys',
      follow: true,
    },

Error: Required field "companyId" in model "Tech" not defined in table item

{
   "model":"Tech",
   "raw":{
      "sk":"Tech#5c479b58-abb0-4158-bf4e-c2332f44940a",
      "getByBatchPk":"Batch#9d37c4a4-f2d7-48bb-9a0f-96a6c2797627",
      "pk":"Company#908d1390-a9a5-474a-9807-76283d7eb443:Tech",
      "getByBatchSk":"Tech:000000001"
   },
   "params":{
      "parse":true,
      "high":true,
      "index":"getByBatch",
      "follow":false,
      "hidden":true,
      "checked":true
   },
   "field":{
      "type":"string",
      "required":true,
      "name":"companyId",
      "isoDates":true,
      "partial":false,
      "attribute":[
         "companyId"
      ],
      "nulls":false
   }
}

I saw there is a noerror param. Will that turn off all errors or just this "not defined" one? As I still want to catch real errors but currently the size of the logs is starting to cost us money as we pay for Kloudmate by the amount of logs ingested.

Otherwise I might have to revert to an older version until this is resolved.

I do feel like this error shouldn't have just been turned on by default, there could have been some flag to switch it on for now that could have been defaulted to on in a later version once any teething problems were resolved.

[AlexStansfield]

Checked the code and noerror seems to only affect this error. However it doesn't work if set in schema, only if set on the find operation itself.

[AlexStansfield]

So this version of the library went onto our production on Wednesday. In the last 24 hours it has produced 480,000 log messages.

My thoughts on this new error:

1. Turn it off by default
2. Make it a warning rather than an error
3. Remove noerror param as naming wise could cover a lot of things
4. Add a new param (e.g warnOnMissingRequired) that is false by default that could be set in params on schema or on operation

In a later release (maybe next major) the warnOnMissingRequired could default to true.

[mobsense]

Sounds like a good suggestion.

We should add to the test to not perform for indexes with projection. But your suggestion to reverse the test to be opt-in would be better.

[mobsense]

There are already a few places in the code that emit warnings if table.warn is true. So we can extend this.

With the proposed patch, you can set:

• Schema.params.warn
• API params.warn

If set (and a whole host of conditions are not true), then a warning will be omitted.

This warning is really important if you update your schema and make some fields required, but don't update all items in the database to define the required field. You want to detect that condition. So that is what the warning is for.

With opt-in, as you suggest, you can turn it on globally via the schema or on a per-API basis.

Thanks for raising the issue.

[mobsense]

Before releasing this, did this patch address the issue?

[AlexStansfield]

Before releasing this, did this patch address the issue?

Which patch?

[mobsense]

The current repo includes the params.warn and turns warnings off by default.