BusinessCentral.LinterCop icon indicating copy to clipboard operation
BusinessCentral.LinterCop copied to clipboard

LC0048 - False positive? Error with GetLastErrorText or Rec."Error Message"

Open pri-kise opened this issue 1 year ago • 6 comments

Is the following line a false positive?

Error(GetLastErrorText());

pri-kise avatar Dec 18 '23 10:12 pri-kise

I've got another one where I'm not sure if this is a false positive or if I'd need a pragma

//Note Method is called from an external Service, but we'd like to protocol the Error on the Record, but return the error to the external service.
procedure DoMagic(EntryNo: Integer)
var
   PTEQueueEntry: Record "PTE Queue Entry"
begin
    PTEQueueEntry.Get(EntryNo);
    PTEQueueEntry.Process();
    Commit();
    if PTEQueueEntry.Get(PTEQueueEntry."Entry No.") and (PTEQueueEntry."Error Message" <> '') then begin
        Error(PTEQueueEntry."Error Message"); // Is this a valid warning for LC00048 or do we need a pragma here?
    end;
end;

pri-kise avatar Dec 18 '23 11:12 pri-kise

I'll have to check, but I'm quite certain these will show up in the telemetry as Use ERROR with a text constant to improve telemetry details.

So i think this is not a false positive, where the rule focuses on having details in the telemetry. To have more details in the telemetry you need to implement the ErrorInfo and optionally determine the DataClassification of the Error message.

procedure MyGetLastErrorText()
var
    MyErrorInfo: ErrorInfo;
begin
    MyErrorInfo := ErrorInfo.Create(GetLastErrorText());
    MyErrorInfo.DataClassification(DataClassification::SystemMetadata);
    Error(MyErrorInfo);
end;
procedure DoMagic(EntryNo: Integer)
var
   PTEQueueEntry: Record "PTE Queue Entry";
   MyErrorInfo: ErrorInfo;
begin
    PTEQueueEntry.Get(EntryNo);
    PTEQueueEntry.Process();
    Commit();
    if PTEQueueEntry.Get(PTEQueueEntry."Entry No.") and (PTEQueueEntry."Error Message" <> '') then begin
        MyErrorInfo := ErrorInfo.Create(PTEQueueEntry."Error Message");
        MyErrorInfo.DataClassification(DataClassification::EndUserPseudonymousIdentifiers);
        Error(MyErrorInfo);
    end;
end;

Do you have access to Telemetry to verify if this in deed the case this? Otherwise I need to setup an environment to verify this, but will take some more time.

Arthurvdv avatar Dec 18 '23 16:12 Arthurvdv

Sadly this app is currently only used in OnPrem an still under development and we don't have telemetry yet for this purpose.

I will ask in yammer for help on this issue.

I've got another code, that I don't know how to refactor:

field(3; "Dimension Code"; Code[20])
{
    Caption = 'Dimension Code';
    TableRelation = Dimension.Code;

    trigger OnValidate()
    begin
        if not DimMgt.CheckDim("Dimension Code") then
            Error(DimMgt.GetDimErr()); //Will ErrorInfo help here?
        "Dimension Value Code" := '';
    end;
}

pri-kise avatar Dec 19 '23 13:12 pri-kise

I've created an environment where I could access the application insight to review the possible scenario's.

To have the content of the message of the error itself in the telemetry, you need to wrap the content (label/text/...) in a ErrorInfo object, see the example below;

field(3; "Dimension Code"; Code[20])
{
    Caption = 'Dimension Code';
    TableRelation = Dimension.Code;

    trigger OnValidate()
    begin
        if not DimMgt.CheckDim("Dimension Code") then
            Error(ErrorInfo.Create(DimMgt.GetDimErr()));
        "Dimension Value Code" := '';
    end;
}

Great example by-to-way. I've taken the liberty to use your example in the wiki on the documentation of this rule: https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0048.

The only thing I'm not sure on howto implement correctly the DataClassification. In the telemetry I find cases like these below, but unsure howto recreate them with the ErrorInfo object.

  • Message not shown because the NavBaseException constructor was used without privacy classification
  • The Item doesn't exist. Identification fields and values: <redacted>

I've try'ed with every possbile DataClassification setting, where at the moment the complete text is always committed to application insights. I'm unfamiliar at this moment howto handle error messages with sensitive information.

Arthurvdv avatar Dec 20 '23 10:12 Arthurvdv

So the implication here is that if I have a function that returns text I need to wrap it inside the ErrorInfo?

What does a Label do differently than a text value from a function?

tscottjendev avatar Jan 15 '24 13:01 tscottjendev

I've try'ed to add context about this in the documentation of the rule itself:

When Analyzing Telemetry with the Dynamics 365 Business Central Usage Analytics report, having access to the correct error message is essential for a effective analysis.

https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0048

Arthurvdv avatar Jan 15 '24 14:01 Arthurvdv