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
657 stars 93 forks source link

Outlook for Windows prompts user in OnMessageSend event when the eventhandler has finished without blocking #3860

Closed ilbrando closed 7 months ago

ilbrando commented 7 months ago

I have an OnMessageSend event handler with SendMode=Block. My handler finishes handling the event in a short time (less than 5 seconds), but in Outlook for Windows the user is presented with a confirmation dialog.

image

I don't think this situation/dialog is mentioned in the documentation. On Outlook for web this dialog is not shown.

Your Environment

Expected behavior

The email to be sent without prompting the user.

Current behavior

The user is prompted though the event handler didn't prevent sending the email.

millerds commented 7 months ago

How are you calling the completed event in your code?

ilbrando commented 7 months ago

Like this:

event.completed({ allowEvent: true });

It works as expected in Outlook for web.

millerds commented 7 months ago

This dialog is shown if the add-in finishes running after having run for a little while. You say it ran for a short time, but it apparently ran just long enough for this dialog to come up. Can you see another message show up briefly before that one (there is one that explains the add-in is running for a long time)? What happens if you adjust your add-in to not run something that is take more of the execution time?

If you don't think the dialog should come up explaining that the add-in finally finishes you can make a feature suggestion at https://aka.ms/M365dev-suggestions

Note that the web implementation may be a little different then the Win32 implementation and timings will definitely be different.

ilbrando commented 6 months ago

I have now timed this. From I click the Send button until the dialog is shown is max 2-3 seconds. My handler only makes one HTTP request (it gets the data from the mail item session data so it isn't spending time calculating data). I can find this request in Azure Application insights. The total amount of time processing the request is 967ms. I'm pretty sure it hasn't always been this sensible. Has something changed in Outlook regarding this?

I have enclosed a video showing what is happing. When the stopwatch is at approx 10 seconds I click the Send button (this is at 00:11 in the video). At 00:14 in the video the dialog is shown. The stopwatch is at 12:87.

https://github.com/OfficeDev/office-js/assets/11632690/895633e0-7891-417b-b3a4-204d5f94b778

millerds commented 6 months ago

Would you mind sharing the HTTP requires code your handler uses (please don't share any personal information in the process)?

ilbrando commented 6 months ago

Sure. Here is the entire code:

// Code build the normal way with webpack doesn't work in Outlook for Windows.
// We can't use imports here (Outlook doesn't support it) so we can't use
// code from our codebase.

const dataItemKey = "advosys-new-mail-data";

function getMailItemDataFromSession(sessionData, resultCallback) {
  sessionData.getAsync(dataItemKey, asyncResult => {
    if (asyncResult.status !== Office.AsyncResultStatus.Succeeded) {
      resultCallback(undefined);
      return;
    }
    const data = JSON.parse(asyncResult.value); // type SendMailData
    resultCallback(data);
  });
}

function setMailItemDataInSession(sessionData, data) {
  const jsonData = JSON.stringify(data);
  sessionData.setAsync(dataItemKey, jsonData);
}

// This code is simple javascript version without any imports of event-handler.ts
function mailSendEventHandler(event) {
  const mailItem = Office.context.mailbox.item;

  getMailItemDataFromSession(mailItem.sessionData, sendMailData => {
    if (sendMailData === undefined || sendMailData === null) {
      // This can happen when there is no data in the session.
      event.completed({ allowEvent: true });
      return;
    }

    switch (sendMailData.state) {
      case "disabled":
        event.completed({ allowEvent: true });
        return;

      case "error":
        if (sendMailData.hasBeenHandled) return;
        // spread operator not supported
        setMailItemDataInSession(mailItem.sessionData, Object.assign({}, sendMailData, { hasBeenHandled: true, timestamp: Date.now(), hasBeenSubmitted: true }));
        event.completed({ allowEvent: false, errorMessage: sendMailData.message });
        return;

      case "ok":
        if (sendMailData.hasBeenHandled) return;
        // we must ensure Outlook has saved the mail before calling the backend
        mailItem.saveAsync(asyncResult => {
          if (asyncResult.status === Office.AsyncResultStatus.Failed) {
            event.completed({ allowEvent: false, errorMessage: "Outlook save mail failed." });
          } else {
            // spread operator not supported
            setMailItemDataInSession(mailItem.sessionData, Object.assign({}, sendMailData, { hasBeenHandled: true, timestamp: Date.now() }));

            const url = `https://${sendMailData.backend.baseUrl}/advosys/mail-management/v4/matter-mails`;

            const xhr = new XMLHttpRequest();
            xhr.open("POST", url);
            xhr.setRequestHeader("tenantId", sendMailData.backend.tenantId);
            xhr.setRequestHeader("Content-Type", "application/json");
            xhr.setRequestHeader("Authorization", sendMailData.backend.headers.Authorization);
            xhr.setRequestHeader("AddIn-Bearer", "");

            xhr.onreadystatechange = () => {
              if (xhr.readyState === XMLHttpRequest.DONE && (xhr.status === 0 || (xhr.status >= 200 && xhr.status <= 399))) {
                event.completed({ allowEvent: true });
              } else {
                event.completed({ allowEvent: false, errorMessage: xhr.responseText });
              }
            };
            xhr.send(sendMailData.saveMailBodyJson);
          }
        });
        return;

      default:
        throw new Error(`Unknown sendMailData.stat: ${sendMailData.state}`);
    }
  });
}

if (Office.context.platform === Office.PlatformType.PC || Office.context.platform == null) {
  Office.actions.associate("mailSendEventHandler", mailSendEventHandler);
}
millerds commented 6 months ago

I'm not sure which code path through this event handler is triggering the behavior you are seeing . . . but I can see a handful of ways in which the code returns without calling event.completed(...) at all, which would trigger the long running add-in behavior.

In the case of your XMLHttpRequest.send() call, which is asynchronous, you call that and then the method calling it ends. The completed event is called in the state change of the send call. I'm not familiar with the state change, but is it possible for the state to change in such a way that you hit the "else" which starts to call the event.completed fail case, but then the state changes again really quickly again that then this the "if" case and calls event.completed true case? Seems like you need to flesh out the state change of the service call and the event.completed call.

ilbrando commented 6 months ago

I expect Outlook to wait for the event.completed callback. How else are you supposed to make async calls in the handler? But now that you make me look at the code again, I can see I miss calling event.completed in the two if statements in the top of the cases of "error" and "ok" :-) The "normal" path through this is "ok" case. All code paths in this calls event.completed.

ilbrando commented 6 months ago

@millerds For your information here is an example from the documentation. It calls Office.context.mailbox.item.location.getAsync and returns immediately.

// The following example checks whether a location is specified in an appointment before it's sent.
function onAppointmentSendHandler(event) {
    Office.context.mailbox.item.location.getAsync({ asyncContext: event }, asyncResult => {
        const event = asyncResult.asyncContext;
        if (asyncResult.status === Office.AsyncResultStatus.Failed) {
            console.log(asyncResult.error.message);
            // If the add-in is unable to retrieve the appointment's location, the appointment isn't sent.
            event.completed({ allowEvent: false, errorMessage: "Failed to get the appointment's location." });
            return;
        }

        if (asyncResult.value === "") {
            // If no location is specified, the appointment isn't sent and the user is alerted to include a location.
            event.completed(
                {
                    allowEvent: false,
                    cancelLabel: "Add a location",
                    commandId: "msgComposeOpenPaneButton",
                    contextData: JSON.stringify({ a: "aValue", b: "bValue" }),
                    errorMessage: "Don't forget to add a meeting location.",
                    sendModeOverride: Office.MailboxEnums.SendModeOverride.PromptUser
                }
            );
        } else {
            // If a location is specified, the appointment is sent.
            event.completed({ allowEvent: true });
        }
    });
}
millerds commented 5 months ago

Yes . . . that sample is able to do an async call with event.completed being called in different cases. Note that all paths through the code end with an event.completed and it is not called more than once.

Outlook is waiting for event.completed. If the add-in runs for a long time before calling event.completed or finishes without calling event.completed then a dialog will come up saying the add-in is running and you need to wait. I'm suggesting that in the case of xhr.send the state is getting changed multiple times as the call is progressing through UNSENT => OPENED => HEADERS_RECEIVED => LOADING => DONE states and each time the state is not "DONE" you are calling event.completed({ allowEvent: false, errorMessage: xhr.responseText }) which is finally followed by the event.completed({ allowEvent: true }) call. If this happens fast enough you wouldn't be able to see the result of the first call, but it would change how the add-in completion is being tracked in Outlook and you get the file "add-in is done" dialog when the final completed call is made. I think if you remove the completed event call from the state change else clause then it would behave as you desire.

ilbrando commented 5 months ago

@millerds Thank you. These are good points. I have tried to change the HTTP request as suggested, and it seems to have removed the problem.