search

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
689 stars 95 forks source link

Attachments intermittently become locked on a message and cannot be deleted or previewed in new outlook #4610

Open rbays opened 5 months ago

rbays commented 5 months ago

Provide required information needed to triage your issue

I believe that this issue may be to do with calling customProps.saveAsync in Office.CustomProperties, but have not been able to come to a concrete conclusion

We have experience an issue locally and in our dev environment where intermittently, after adding attachments to a new email, these attachments will become impossible to remove, only in new outlook.

Your Environment

Expected behavior

Attachments should be deleteable and previewable whenever added

Current behavior

We have experience an issue locally and in our dev environment where intermittently, after adding attachments to a new email, these attachments will become impossible to remove, only in new outlook.

Steps to reproduce

  1. Open a new email to compose
  2. Add attachments
  3. Cause something to update a custom prop on the message (maybe, I'm not sure if this is the cause, we have an "onMessageCompose" event which may be the culprit, but I can only replicate the issue 1/50 times as I think there is a race condition)
  4. Attempt to delete the email.

Link to live example(s)

Provide additional details

Context

our addin needs to delete existing attachments, but is unable to since outlook is convinced that they don't exist

Useful logs

I have a .har of the web request being made when I most recently replicated the issue, but cannot upload here I also have a video of the issue happening, but do not want to upload to a public repo as it contains sensitive data

image

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.

isabela-dominguez commented 4 months ago

Thank you for sharing this issue @rbays! Connecting you @exextoc with who may be able to help.

Swathy-Mothilal commented 4 months ago

@rbays We are unable to repro the issue locally. Can you please host your repro video and the har logs to a private repo, and grant permission to access it?

rbays commented 4 months ago

Hi @Swathy-Mothilal I have invited you as a collaborator to the private repo, is there anyone else I need to add too?

Swathy-Mothilal commented 4 months ago

@rbays Thanks for sharing the repo. Could you please provide a video demonstrating the issue and logs? We need to understand the flow to be able to reproduce it.

Swathy-Mothilal commented 4 months ago

@rbays Please provide a working manifest link also where you are able to repro the issue.

rbays commented 4 months ago

Hi, I don't have a manifest available to share, but I believe I have managed to isolate the issue. The below code will cause the issue on New Outlook for windows (there is no issue on old outlook or web)

the processMessageComposeEvent event is in the manifest as a launch event as follows <LaunchEvent FunctionName="processMessageComposeEvent" Type="OnNewMessageCompose"/> 

Office.initialize = () => { };

type MailboxItem = Office.Item & Office.ItemCompose & Office.ItemRead &
Office.Message & Office.MessageCompose & Office.MessageRead &
Office.Appointment & Office.AppointmentCompose & Office.AppointmentRead;
function getContext() : Office.Context {
    if (Office.context) {
        return Office.context;
    }
    throw new Error();
}
function getMailbox() : Office.Mailbox {
    const result = getContext().mailbox;
    if (result) {
        return Office.context.mailbox;
    }
    throw new Error();
}

function getMailboxItem() : MailboxItem {
    const result = getMailbox().item;
    if (result) {
        return result;
    }
    throw new Error();
}
async function saveCustomProperties(customProps: Office.CustomProperties) : Promise<void> {
    return new Promise((resolve, reject) => {
        customProps.saveAsync((asyncResult) => {
            if (asyncResult.status === Office.AsyncResultStatus.Failed) {
                reject(asyncResult.error);
            } else {
                resolve();
            }
        });
    });
}
function getItemCustomPropertiesAsync(): Promise<Office.CustomProperties> {
    return new Promise((resolve, reject) => {
        getMailboxItem().loadCustomPropertiesAsync((asyncResult) => {
            if (asyncResult.status === Office.AsyncResultStatus.Failed) {
                reject(asyncResult.error);
            } else {
                resolve(asyncResult.value);
            }
        });
    });
}
async function setItemCustomPropertyAsync(key: string, value: string): Promise<void> {
    const customProps = await getItemCustomPropertiesAsync();
    customProps.set(key, value);

    return saveCustomProperties(customProps);
}

async function processMessageComposeEvent(): Promise<void> {
    let x = 0;
    const intervalID = setInterval(() => {
        x += 1;
        if (x >= 40) {
            window.clearInterval(intervalID);
        } else {
            setItemCustomPropertyAsync('test_key_iterator', x.toString());
        }
    }, 500);
}

export default processMessageComposeEvent;

The issue seems to be some kind of race condition between customProps.SaveAsync and whatever is happening with the attachment being saved in office.

The code above just calls that method every half a second for the first 20 seconds when you start composing a new email.

By opening a new email, and then attaching some small attachments withing the first 5-10 seconds of having the message open I have been able to consistently reproduce the issue.

Please let me know if you need any more help to replicate the issue

Swathy-Mothilal commented 4 months ago

@rbays , Thanks for sharing the code snippet. In your code, the processMessageComposeEvent function sets custom properties in a loop with a 500ms interval. If the attachments are being saved at the same time, there might be a conflict or timing issue that causes the attachments not to be removed properly. To avoid this race condition, could you please wait for Attachments to be Saved. Ensure that the attachments are fully saved before starting the custom property setting operations.

Here is a code snippet, that can help you. async function waitForAttachmentsToBeSaved(): Promise { return new Promise((resolve) => { const checkAttachments = () => { const attachments = getMailboxItem().attachments; if (attachments.every(attachment => attachment.isInline || attachment.isSaved)) { resolve(); } else { setTimeout(checkAttachments, 100); } }; checkAttachments(); }); }

Please let me know if you have any queries.

rbays commented 4 months ago

Hi @Swathy-Mothilal ,

I don't have the isSaved property available to me, I have upgraded the package https://www.npmjs.com/package/@types/office-js to 1.0.401 (which is currently the latest) and only have the following props available on the AttachmentDetails interface that is returned.

image

Could you please advise if I should be using a different package to access this property?

Further, even if this does fix my issue, I believe that this will cause issues for others in the future, this is only an issue on New outlook (not OWA or old outlook) and means that every time someone saves a custom prop, they run the risk of breaking attachments. My example runs on a loop to ensure I can recreate the issue consistently, but when I initially encountered the issue, it was happening sporadically during the normal course of usage. As such I believe that this is a bug that should be fixed by the office-JS team

rkpathak commented 4 months ago

@rbays Few observations for your handler function in the code that you shared.

  1. event.completed() is not being called in the handler, without that function will not work properly.
  2. The handler code is returning a promise that platform can't wait for. The handler function should follow the rules mentioned at https://learn.microsoft.com/en-us/office/dev/add-ins/outlook/autolaunch#event-based-activation-behavior-and-limitations

Can you create a repro script in script lab, to help repro this.

rbays commented 4 months ago

Hi @rkpathak ,

1) I have amended the process event to call the event.completed method below, though it doesn't make a difference and still has issues in the same way 2) I cannot see anything relevant in the section you linked, but do you mean that the function should not be asynchronous?

async function processMessageComposeEvent(event: any): Promise<void> {
    let x = 0;
    const intervalID = setInterval(() => {
        x += 1;
        if (x >= 40) {
            window.clearInterval(intervalID);
        } else {
            setItemCustomPropertyAsync('test_key_iterator', x.toString());
            if (x === 39) {
                event.completed();
            }
        }
    }, 500);
}
rkpathak commented 4 months ago

@rbays Can you create a repro script in script lab and share the link, to help repro this.

rbays commented 4 months ago

@rkpathak I have created a script in script lab, however I have not been able to repro the issue this way because as far as I can tell, script lab does not run on new outlook.

I took an existing template and replaced the main function with the exact code from the example that I provided earlier.

I hope this helps.

Here are the segments of the script:

Script

$("#wreck-it").on("click", processMessageComposeEvent);

type MailboxItem = Office.Item &
  Office.ItemCompose &
  Office.ItemRead &
  Office.Message &
  Office.MessageCompose &
  Office.MessageRead &
  Office.Appointment &
  Office.AppointmentCompose &
  Office.AppointmentRead;

function getContext(): Office.Context {
  if (Office.context) {
    return Office.context;
  }
  throw new Error();
}
function getMailbox(): Office.Mailbox {
  const result = getContext().mailbox;
  if (result) {
    return Office.context.mailbox;
  }
  throw new Error();
}

function getMailboxItem(): MailboxItem {
  const result = getMailbox().item;
  if (result) {
    return result;
  }
  throw new Error();
}
async function saveCustomProperties(customProps: Office.CustomProperties): Promise<void> {
  return new Promise((resolve, reject) => {
    customProps.saveAsync((asyncResult) => {
      if (asyncResult.status === Office.AsyncResultStatus.Failed) {
        reject(asyncResult.error);
      } else {
        resolve();
      }
    });
  });
}
function getItemCustomPropertiesAsync(): Promise<Office.CustomProperties> {
  return new Promise((resolve, reject) => {
    getMailboxItem().loadCustomPropertiesAsync((asyncResult) => {
      if (asyncResult.status === Office.AsyncResultStatus.Failed) {
        reject(asyncResult.error);
      } else {
        resolve(asyncResult.value);
      }
    });
  });
}
async function setItemCustomPropertyAsync(key: string, value: string): Promise<void> {
  const customProps = await getItemCustomPropertiesAsync();
  customProps.set(key, value);

  return saveCustomProperties(customProps);
}

async function processMessageComposeEvent(): Promise<void> {
  let x = 0;
  const intervalID = setInterval(() => {
    x += 1;
    if (x >= 40) {
      window.clearInterval(intervalID);
    } else {
      setItemCustomPropertyAsync("test_key_iterator", x.toString());
      console.log(x);
    }
  }, 500);
}

HTML

<section class="ms-font-m">
    <p class="ms-font-m">This sample saves custom props a bunch to try to break attachments</p>
    <p><b>Required mode</b>: Compose</p>
</section>

<section class="samples ms-font-m">
    <h3>Try it out</h3>
    <ol>
        <li>click the button</li>
        <br>
        <li>add some attachments within 20 seconds</li>
        <br>
        <li>watch them break</li>
    </ol>
    <div class="ms-TextField">

        <button id="wreck-it" class="ms-Button">
        <span class="ms-Button-label">I'm gonna wreck it!</span>
    </button>
</section>

CSS

section.samples {
    margin-top: 20px;
}

section.samples .ms-Button, section.setup .ms-Button {
    display: block;
    margin-bottom: 5px;
    margin-left: 20px;
    min-width: 80px;
}

Libraries

https://appsforoffice.microsoft.com/lib/1/hosted/office.js
@types/office-js

office-ui-fabric-js@1.4.0/dist/css/fabric.min.css
office-ui-fabric-js@1.4.0/dist/css/fabric.components.min.css

core-js@2.4.1/client/core.min.js
@types/core-js

jquery@3.1.1
@types/jquery@3.3.1
rbays commented 4 months ago

@rkpathak is there any update or timeline on this?

kumarshrey-msft commented 4 months ago

Hi @rbays, apologies for the delay. I was able to reproduce this issue on the New Outlook, however as you mentioned this is very intermittent. Can you briefly elaborate the use case you are trying to address? If your add-in depending on attachments being saved or removed so that you can subsequently save the custom properties, you can consider listening to the OnMessageAttachmentsChanged event instead of doing so after intervals of 500ms?

rbays commented 4 months ago

Hi @kumarshrey-msft, the example I gave here isn't my actual use case, it's an example I created to help to hit the problem with more consistency to help you debug the problem.

In our actual usage we are storing a custom property on the message customProps when the user clicks a button in our taskpane to signify how the message should be processed on send, and also we are saving another custom prop during the messagecompose event to keep reference to the message.

The problem we have is that if the user happens to add an attachment to the email close to either of these events happening, there is a chance of the attachments breaking, which our QA team found, and managed to replicate about 1/10 times when trying.

As far as I can tell at the moment in new outlook, if you ever want to save message customProps, and the user wants to add an attachment, there is a chance that these 2 actions will clash and break the attachments for the user.

kumarshrey-msft commented 4 months ago

@rbays Thanks for reporting this issue, we have been able to reproduce it and it has been put on our backlog. We unfortunately have no timelines to share at this point.

Internal tracking id: Office: [4694943]

patilganesh-msft commented 1 month ago

Hi @rbays,

This issue is mostly happening because of the mail item is being saved by 2 entities (removeAttachments and setCustomProperties).S, sometimes there is a conflict at the server side. While we carry on the server side investigation, could you please implement the logic of retries while removing the attachments? Run a loop for 3 retries, if successful exit of the loop, else delay then next retry in 1 or 2 seconds. This should resolve the issue. Hope it helps. We will keep you updated with the server side investigation.

patilganesh-msft commented 3 weeks ago

I checked with the server team as well. They suggested that the irresolvable conflicts need to be handled by add-ins only. So, kindly implement the retry logic as I suggested and let me know if it solves/doesn't. Happy to assist further.
This should solve the issue. Thank you.

rbays commented 3 weeks ago

Hi @patilganesh-msft, I don't know what you want me to retry.

Could you please be more specific as to what I should retry where please?

patilganesh-msft commented 2 weeks ago

@rbays Ok, understood your issue that the attachment is being removed from the UI and not via add-in code. So it looks similar to the issue of attachment id getting changed after save operation for which already a bug has been opened with the attachments team and currently, we do not have ETA for the resolution.