OfficeDev / office-js

A repo and NPM package for Office.js, corresponding to a copy of what gets published to the official "evergreen" Office.js CDN, at https://appsforoffice.microsoft.com/lib/1/hosted/office.js.
https://learn.microsoft.com/javascript/api/overview
Other
667 stars 94 forks source link

Outlook AddIn 'OnMessageAttachmentsChanged' removeAttachmentAsync fails when you attach 3 or more files #4627

Open guzman-rc opened 2 months ago

guzman-rc commented 2 months ago

We have developed an addin that is triggered when a file is attached, for this we use the event: 'OnMessageAttachmentsChanged'.

When a file is attached the following processes occur:

  1. Download the attached file.
  2. Process its metadata.
  3. Delete the attached file.
  4. Attach the file with the processed metadata.

The problem is that when 3 or more files are attached, the call to the function 'removeAttachmentAsync' fails for some files, it does not return any result and the execution does not continue for that file.

Your Environment

Expected behavior

Everything works correctly when attaching a single file, when attaching 3 or more the call to removeAttachmentAsync fails for some of the files resulting in those files not being attached with the metadata processed.

Current behavior

The call to removeAttachmentAsync removes the attached file but terminates the rest of the pending processes which causes the new file with the processed metadata not to be attached.

Link to live example(s)

  1. Install the Addin from the url: https://www.adarsus.com/addin/outlook_event/manifest.xml
  2. Access your email account from Chrome and activate the log console.
  3. Attach a file and view log sequence.
  4. Attach 3 or more files and check that the process fails for several files.

Useful logs

Thank you for taking the time to report an issue. Our triage team will respond to you in less than 72 hours. Normally, response time is <10 hours Monday through Friday. We do not triage on weekends.

Swathy-Mothilal commented 2 months ago

@guzman-rc , could you please provide a video demonstrating the issue you're experiencing, along with the logs, to help us better understand the problem?

guzman-rc commented 2 months ago

I will try to create a video, in the meantime I will explain the operation of the example Addin.

The objective of this Addin is to eliminate the metadata of the attached files, for this purpose, for each file that the user attaches the following tasks will be carried out:

  1. OnMessageAttachmentsChanged event identifies when the file has finished attaching.
  2. Download the contents of the attached file.
  3. Remove metadata from the downloaded file.
  4. Remove the attached file.
  5. The file without metadata is attached.

See log output when attaching file 1.pdf

Captura

The error occurs when we attach 3 or more files at the same time.

We have detected that there is a problem with the function: removeAttachmentAsync, it does not remove the file and ends the process.

rkpathak commented 1 month ago

@guzman-rc Can you share the handler function that is getting called for the event?

guzman-rc commented 1 month ago

You can install the Addin to test it, the manifest file is available from the url: https://www.adarsus.com/addin/outlook_event/manifest.xml

All the code is available from the file: https://www.adarsus.com/addin/outlook_event/commands.js

The function called for the OnMessageAttachmentsChanged event is: onMessageAttachmentsChangedHandler

kumarshrey-msft commented 1 month ago

@guzman-rc From my understanding this is the same issue like #4646 where the handler for OnMessageAttachmentsChanged itself is not getting triggered in certain scenarios when multiple files are attached, and is not specific to removeAttachmentAsync API as such, can you please confirm the same? If yes, we can close this issue as duplicate.

guzman-rc commented 1 month ago

Hi,

No, it is not the same scenario.

In issue #4627 it is reported that when attaching 3 or more files the removeAttachmentAsync function fails.

In issue #4646 it is reported that OnMessageAttachmentsChanged event is fired only for the first file when the email is created in a new window (Popup window).

Best regards,

From: Kumar Shrey @.> Date: Wednesday, 17 July 2024 at 14:35 To: OfficeDev/office-js @.> Cc: guzman-rc @.>, Mention @.> Subject: Re: [OfficeDev/office-js] Outlook AddIn 'OnMessageAttachmentsChanged' removeAttachmentAsync fails when you attach 3 or more files (Issue #4627)

@guzman-rchttps://github.com/guzman-rc From my understanding this is the same issue like #4646https://github.com/OfficeDev/office-js/issues/4646 where the handler for OnMessageAttachmentsChanged itself is not getting triggered in certain scenarios when multiple files are attached, and is not specific to removeAttachmentAsync API as such, can you please confirm the same? If yes, we can close this issue as duplicate.

— Reply to this email directly, view it on GitHubhttps://github.com/OfficeDev/office-js/issues/4627#issuecomment-2233216488, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AXBZDDQW6MB6X7QQDRNAFM3ZMZQHXAVCNFSM6AAAAABKFZW5V6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZTGIYTMNBYHA. You are receiving this because you were mentioned.Message ID: @.***>

kumarshrey-msft commented 1 month ago

Hi @guzman-rc, I am able to reproduce the issue with the manifest you shared, however, I don't see it as a shortcoming of the removeAttachmentAsync API in itself. Consider the below example:

const attachments = [
    {"id":"AAkALgAAAAAAHYQDEapmEc2byACqAC/EWg0AF1DUX4ZWa0KEdGEzihNfpQACYSmLqQAAARIAEACiGPuEZvAmQZTeWzmA4gqn","name":"img1.png","contentType":"image/png","size":481031,"attachmentType":"file","isInline":false},
    {"id":"AAkALgAAAAAAHYQDEapmEc2byACqAC/EWg0AF1DUX4ZWa0KEdGEzihNfpQACYSmLqQAAARIAEABINoB+ZcOKQo3CwahJs/gU","name":"img2.png","contentType":"image/png","size":587031,"attachmentType":"file","isInline":false},
    {"id":"AAkALgAAAAAAHYQDEapmEc2byACqAC/EWg0AF1DUX4ZWa0KEdGEzihNfpQACYSmLqQAAARIAEAAUYVwHLqpaSKUiQ56g4KtN","name":"img3.png","contentType":"image/png","size":998554,"attachmentType":"file","isInline":false}
]

for (const item of attachments) {
    const id = item.id;
    // Remove an attachment
    Office.context.mailbox.item.removeAttachmentAsync(
        id,
        {
            "asyncContext": { foo: 6, bar: 28 }
        },
        function (asyncResult) {
            showMessage(JSON.stringify(asyncResult));
        }
    );
}

In this case, the removeAttachmentAsync API is successfully able to remove all the 3 files (and correspondingly, three OnMessageAttachmentsChanged events are fired for the corresponding files, with attachmentStatus set to removed. I believe you are running into a race condition due to the parallel flow of execution. I would suggest you re-look the way you are handling the asynchronous operations. Maybe instead of immediately reacting to OnMessageAttachmentsChanged finishing attaching the file, add a delay, process multiple such events together, and then correspondingly trigger the removeAttachmentAsync API.

Let me know your thoughts.

guzman-rc commented 1 month ago

Hi @kumarshrey-msft,

For each OnMessageAttachmentsChanged event of type Added the following tasks are executed:

  1. Download the contents of the attached file.
  2. Remove metadata from the downloaded file.
  3. Remove the attached file.
  4. The file without metadata is attached.

As you can see, from the moment the OnMessageAttachmentsChanged event is fired until the removeAttachmentAsync function is invoked there is already a delay in the execution of the tasks:

  1. Download the contents of the attached file and
  2. Remove metadata from the downloaded file.

You propose to add an extra delay before invoking the removeAttachmentAsync function, how long?

DivyaPatidar commented 1 month ago

@guzman-rc As a workaround can you try to ensure that removeAttachmentAsync calls are made sequentially from your side and see if that solves the issue?

guzman-rc commented 1 month ago

It is not possible to execute sequentially removeAttachmentAsync, the processes:

DivyaPatidar commented 1 month ago

Yes, I understand that. By sequential I mean using promise or something similar to ensure you make only one call to removeAttachmentAsync at a time.

guzman-rc commented 1 month ago

It should not be a problem to invoke removeAttachment simultaneously, just make sure that the file has been attached successfully before you can invoke removeAttachment.

rajjha-msft commented 5 days ago

Hey @guzman-rc

Thank you for reporting the issue, we've put it in our backlog. Unfortunately, we do not have any timelines to share at this point.

Internal tracking Id : 302409