Inconsistent data returned from hooks *POTENTIAL BUG*

[wrobson-lllow]

This is something I have come across several times over the last several months, but has never been a big issue, and typically the fix is as easy as checking the type before accessing the value. However, it definitely seems off...

I have a CollectionAfterChangeHook on my "license" collection where I need to access the value for "customer" (in this case "DDD") however depending where the hook is invoked changes the format of the data. The following data is from console.logging the "doc" arg:

If the hook is invoked from a direct change on the "license" collection (i.e. a user updates a value manually) the following is passed to the hook as the "doc" argument:

doc: {
  id: '64e66538d008409ec5830cc4',
  customer: 'DDD',
  server: 'DDD_test-server-d',
  product: 1,
  database: 'test-db',
  tier: 'A',
  options: { attachments: true, futureFeature: false },
  createdAt: '2023-08-23T19:59:52.796Z',
  updatedAt: '2023-08-23T20:05:57.808Z'
}

However, I have a CollectionAfterDeleteHook on a separate collection that updates the "options" field on the "licenses" collection. When the "licenses" CollectionAfterChangeHook is invoked via a change made by the CollectionAfterDeleteHook the following is what is passed to the hook as the "doc" argument:

 doc: {
  id: '64e66538d008409ec5830cc4',
  customer: {
    id: 'DDD',
    name: 'test d',
    type: 'school',
    county: {
      id: 'IN-2',
      state: 'IN',
      name: 'Allen',
      code: 2,
      timezone: 'US/Eastern'
    },
    createdAt: '2023-08-09T16:47:52.824Z',
    updatedAt: '2023-08-09T16:47:52.824Z'
  },
  server: {
    id: 'DDD_test-server-d',
    customer: 'DDD',
    machineName: 'test-server-d',
    createdAt: '2023-08-11T18:57:58.606Z',
    updatedAt: '2023-08-11T18:57:58.606Z'
  },
  product: { id: 1, name: 'Financial System' },
  database: 'test-db',
  tier: 'A',
  options: { attachments: false, futureFeature: false },
  createdAt: '2023-08-23T19:59:52.796Z',
  updatedAt: '2023-08-23T20:06:03.449Z'
}

[denolfe]

It looks like the depth parameter is differing between the calls. Are you explicitly setting depth in your calls at any point?

[wrobson-lllow]

No I am not. And as I said both outputs shown above are from the same hook., which I would expect to have the same output.

[wrobson-lllow]

@denolfe Came across the same issue, again... I have a CollectionAfterChange hook on my "messages" collection. "Message" records are created dynamically from other hooks, and some callbacks from an external system, and when the app is in production they will rarely if ever be created manually. However, today I created a message manually and my hook did not run, upon further investigation I realized it was the same issue as before:
Returned "doc" from manually creating message:

doc {
  id: '64edf7f1605cb541757eccbe',
  customer: 'CCC',
  event: 'dispatchFeatureDeploy',
  code: 'ccc',
  status: 'in_process',
  message: {
    tag: '0.0.10',
    stage: 'prod',
    feature: 'attachments',
    corpCode: 'xxx,
    accountId: 'xxxxxxxxxxxxxx',
    accountType: 'xxxxxxx',
    correlationId: 'xxxxxxxxxxxxxxxxxx',
    awsAccountRecordId: 'xxxxxxxxxxxxxxx',
    originalMessageType: 'xxxxxxx'
  },
  createdAt: '2023-08-29T13:51:45.925Z',
  updatedAt: '2023-08-29T13:51:45.925Z'
}

Returned "doc" from dynamically creating same message:

doc {
  id: '64edf924a3f22971c3d90cd4',
  customer: {
    id: 'CCC',
    name: 'test c',
    type: 'school',
    county: {
      id: 'IN-3',
      state: 'IN',
      name: 'Bartholomew',
      code: 3,
      timezone: 'US/Eastern'
    },
    createdAt: '2023-08-07T13:54:25.876Z',
    updatedAt: '2023-08-07T13:54:25.876Z'
  },
  event: {
    id: 'dispatchFeatureDeploy',
    description: 'Feature deploy workflow (currently only "low-attachments") will be dispatched on the creation of this message, either for a feature enabled or for a feature  updated on the existing account.',
    queue: {
      id: 'infrastructureAutomation',
      description: 'Event messages that should result in infrastructure being provisioned should be placed in this queue.',
      settings: [Object],
      _status: 'published',
      createdAt: '2023-08-01T14:43:14.987Z',
      updatedAt: '2023-08-01T14:43:14.987Z'
    },
    priority: 5,
    messageSchema: {
      tag: [Object],
      stage: [Object],
      feature: [Object],
      corpCode: [Object],
      accountId: [Object],
      accountType: [Object],
      correlationId: [Object],
      awsAccountRecordId: [Object],
      originalMessageType: [Object]
    },
    _status: 'published',
    createdAt: '2023-08-17T16:41:41.179Z',
    updatedAt: '2023-08-17T16:41:41.179Z'
  },
  code: 'ccc',
  status: 'in_process',
  message: {
    corpCode: 'xxxxxx',
    accountId: 'xxxxxxxxxxxxxxxxxxx',
    correlationId: 'xxxxxxxxxxxxxxxxxxx',
    awsAccountRecordId: 'xxxxxxxxxxx',
    originalMessageType: 'xxxxxxxxxx',
    accountType: 'xxxxxxxxxxxx',
    tag: xxxxx,
    feature: 'xxxxxxxxxx',
    stage: 'xxxxx'
  },
  createdAt: '2023-08-29T13:56:52.639Z',
  updatedAt: '2023-08-29T13:56:52.639Z'
}

[wrobson-lllow]

You previously asked if I was explicitly setting the "depth" parameter, which I am not. But even if I was (I'm not sure how I would as "doc" is an argument provided by payload) I would still expect that "doc" would be consistent , as I do not have a way to differentiate between a manually and programmatically created message from within the hook (If there is a way to do this, it is not included in the documentation, and I wouldnt see much of a benefit).

[denolfe]

Let me convert this to a possible issue, so it can get looked at.

[MarkAtOmniux]

I can second this issue. Happens as well when checking user in access control, takes a second or a refresh for the full object to pop through, otherwise checking roles can cause the admin panel to crash without proper null checking

[JarrodMFlesch]

@MarkAtOmniux @wrobson-lllow We recently rolled out a fix for this issue but are going to be reverting it in 2.0.

The admin panel uses depth=0 when submitting data, hooks will need to account for any depth by type checking the relationship and fulfilling them if needed.

[wrobson-lllow]

I'm not sure I know what you mean by fulfilling relationships. Also, with the issues I've had I haven't been specifically setting the depth anywhere.

[JarrodMFlesch]

I understand you are not setting depth anywhere, but the admin panel sets depth to 0 on all the action endpoints set on <form> elements. So that is why you are seeing a difference, the admin panel submits with depth=0 (we are setting this) and your payload functions are defaulting to 2.

Regarding fulfilling, I mean if typeof relationshipField === 'string' then you would need to use payload.findByID to resolve that doc.

[wrobson-lllow]

Any reason why the default for the admin panel and the default for the payload functions are not the same?

[jmikrut]

Yes, the reason is because the admin UI explicitly needs different depths based on different contexts.

The admin UI doesn’t actually have a “default” depth - we are explicitly asking for certain depths.

For example, in the List view, we fetch with depth=0 for performance.

Let’s consider if you were in the List view, with 100 per page, and each of your documents had ~20 relationships. If we did not specify depth=0, that would mean at a minimum, 2,000 docs would need to be loaded for just those 100 docs on the page.

But we only need the JSON for the main 100 docs.

This is what the depth property is -for-. It allows us to be efficient with queries and return only what we need in the admin UI.

Making the admin UI match the default local API would be just “covering up” an issue in one place, while it still exists in others simply because the admin UI is not the only place that you might end up setting depth=0.

It’s not a true solution to the problem. Do you see what I’m saying?

Happy to keep the conversation going here!

[wrobson-lllow]

Yes, that makes sense, however, it definitely seems to be done in a way that opens up the potential for issues in a lot of other areas, especially when the documentation for hooks does not mention anything about type checking data. And to use your example of a doc with 20 relationships, that potentially means making 20 extra localAPI calls in a hook, seems inefficient or at the very least inconvenient.

I agree with you that making the default depths the same value is not a true solution to the problem, but it seems like the problem exists nonetheless.

[ssyberg]

I'm still not quite following what the proper way to handle this, I had a filed a bug pre-2.0 related to this and as it happens a bunch of my hooks did break on 2.0 upgrade. I'm going to try this again with the beforeOperation, but some official guidance would be awesome!