Add file name to alert "Donor Ids file was present but it was empty"

[zabeen]

Alert from CheckDonorIdsFromFile Donor Ids file was present but it was empty. at Atlas.DonorImport.Services.DonorChecker.LazilyParsingDonorIdFile.ReadLazyDonorIds()+MoveNext() in /home/vsts/work/1/s/Atlas.DonorImport/Services/DonorChecker/DonorIdCheckerFileParser.cs:line 41 at MoreLinq.MoreEnumerable.<>c__DisplayClass23_0`2.<g__Batch|2>d.MoveNext() at Atlas.DonorImport.Services.DonorChecker.DonorIdChecker.CheckDonorIdsFromFile(DonorIdCheckFile file) in /home/vsts/work/1/s/Atlas.DonorImport/Services/DonorChecker/DonorIdChecker.cs:line 57

• Above is the current alert description, but it's not very helpful.
• The name "Donor Ids file was present but it was empty." is fine, but alert Description needs to be changed.
• Description should list the name of the donor ID checker file that failed.
• Do not need to see the exception message, as we can get that from AI logs if necessary.

Update:

• Alert is raised in response to EmptyDonorFileException
• Exception is caught and suppressed so no info is written to A.I., but that is ok because it is a custom exception we only throw when underlying file stream is null, so stack trace is not useful
• This is used in three kinds of file import: donor, donor id checker and donor info checker
• In all 3 cases, the stack trace is used as alert description without file name
• Proposed fix should be applied to all three cases

[SergeyEzh]

@zabeen, as far I understood, currently exception is suppressed and only exception message is written to AI without stacktrace. Do you want keep it as is but extend AI trace message with stack trace OR make throw it upper to allow AI catch it and log an exception?

[zabeen]

@SergeyEzh As discussed over teams, stack trace isn't useful here as we know exactly what causes EmptyDonorFileException to be thrown (underlying file stream is null).

Further, I see three places in code where EmptyDonorFileException is being thrown and caught:

• Donor id checker file import
• Donor info checker file import
• Donor file import

and in each case the stack trace is being written to the alert message without file name.

So I am extending this ticket to cover all these cases.

[SergeyEzh]

@zabeen, I was doing final testing before commit and now alert message is confusing me. Because it states that file was found but it is empty, though I found that this exception is thrown when I put wrong url to blob to service bus message. When I upload 'real' empty file to blob storage and put correct blob url in service bus it just reads empty file and throws no exception.

[zabeen]

@SergeyEzh In real usage, I have only seen this error where a file was dropped into blob storage, which caused an import request message to be published, and then user manually deleted the file from storage before the donor import app imported it. The blob stream generated by the function is null and then hits the "empty file" exception (which is really a "missing file").

I am surprised an actual empty file doesn't result in any exception though. Perhaps the check that throws the EmptyDonorFileException needs to be extended to stream is null or empty?

[SergeyEzh]

@zabeen, I would prefer two messages like 'Unable to read file content' in case stream is null and 'The file contains no data' in case no data parsed from stream, rather than checking stream length.

[zabeen]

I would prefer two messages like 'Unable to read file content' in case stream is null and 'The file contains no data' in case no data parsed from stream, rather than checking stream length.

@SergeyEzh I'll create a new ticket for that work, right now I want to close and deploy this ticket with the existing support enhancement.

[zabeen]

Testing Notes

Feature test

• First stop the donor import functions app
• Upload a valid donor import file.
• After a new message has been published to donor-import-file-uploads for that file, manually delete the donor import file from the blob storage container.
• Start the donor import functions app
• Outcomes:
  • Alert should be published saying "file was present but it was empty" and name of the affected file.

Repeat same test with:

• donor id checker file
• donor info checker file

Regression tests

• run donor import of new donor
• run donor id check
• run donor info check

[DmitriyShcherbina]

@zabeen Feature testing status: Ok

Regression testing status: Ok