microsoft / microsoft/BCApps

[Bug]: APIV2 LoadFields at OnAfterGetRecord leads to JIT Load Error in Telemetry

Open
#8,709 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Approved Ownership: Manual Team: Integrations
Dominant language
AL
Stars
683
Forks
459
Avg merge
3d 26m
Merged PRs (30d)
633

Description

Describe the issue

We see on the endpoint APIV2/SalesInvoices sometimes the JIT Load error for the fields "Currency Code" or "Amount".
In comparison to the number of successfull requests they are less than a tenth of a percent.

After Looking into the page 30012 "APIV2 - Sales Invoices" I discovered that these fields are loaded with Rec.LoadFields in the trigger OnAfterGetRecord. Which I assume leads to this race condition I see in my customer's temeletry.

https://github.com/microsoft/ALAppExtensions/blob/c56d2785c2ee11c4c1ae35a9d67da08da757e509/Apps/W1/APIV2/app/src/pages/APIV2SalesInvoices.Page.al#L775

Wouldn't it be better to change this loading to reading the data immediately? So I propose to change the LoadField which is called from trigger OnAfterGetRecord to an AddLoadFields in the OnOpenPage trigger where it should be added to the visible fields and thus directly loaded. Since there is no condition why the OnAfterGetRecord is not executing the LoadField command I assume this would be better than loading the fields afterwards.

There are simular implementations other pages as well but with less fields.
page 30051 "APIV2 - Bank Accounts"
page 30038 "APIV2 - Sales Credit Memos"
page 30037 "APIV2 - Sales Quotes"
page 30042 "APIV2 - Purchase Invoices"
page 30083 "APIV2 - Purchase Credit Memos"

Expected behavior

Eliminating the JIT Load error in race conditions.

Steps to reproduce

Create a large number of sales invoice records. Write a routine to update these records with different currency codes and shuffle them in the records.

Read the data with the endpoint salesinvoices and hope to fall into the race condition gap.

Additional context

It should even be possible just to add the AddLoadFields commend because the LoadFields will not JIT load fields which are already loaded.

I will provide a fix for a bug
  • I will provide a fix for a bug

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the linked APIV2SalesInvoices.Page.al location around line 775, then compare the listed APIV2 page implementations for similar LoadFields calls. Use the described concurrent record-update and salesinvoices-read scenario to investigate the telemetry error. Done means the affected fields are loaded without the reported JIT Load race condition across the identified pages.

Written by the indexing model from the issue text.

Assessment

Domain
api, backend-api-design
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.