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
658 stars 94 forks source link

Large matrix dataset infinitely stuck at #BUSY in Excel for the Web #2786

Open wh1t3cAt1k opened 1 year ago

wh1t3cAt1k commented 1 year ago

Returning a large dataset from a custom function makes the function infinitely stuck at #BUSY.

Your Environment

Expected behaviour

The dataset returns completely.

Current behaviour

The function is infinitely stuck at #BUSY, no console errors, nothing.

Steps to reproduce

  1. Use https://github.com/VelixoSolutions/reproduce-large-dataset-busy-bug
  2. Invoke CONTOSO.ANSWER()
  3. Observe #BUSY that never ends.

There are no meaningful errors from the console to report.

Context

Our clients sometimes need to download large datasets from their ERP systems to then aggregate and pivot them at the Excel level. The minimal repro repository in this bug emulates the GI() function in Velixo add-in.

It is one of our most popular custom functions now and it's business-critical for us to support returning large datasets from that function.

The same function works fine in our legacy XLL add-in.

Also, the provided example works in Excel Desktop for Windows.

Macintosh might be affected by the bug, too - please check!

Related issue where, reportedly, payload batching was introduced: https://github.com/OfficeDev/office-js/issues/2222 However, in this case we might be hitting another unidentified bottleneck?

rundongmsft commented 1 year ago

Hi @wh1t3cAt1k, we have reproduced this issue. And we are finding the root cause. If there is progress, we will update it in time.

rundongmsft commented 1 year ago

Hi @wh1t3cAt1k, the function was stuck at "#BUSY" is because the request payload size has exceeded the limit. You can refer to this documentation: "https://docs.microsoft.com/office/dev/add-ins/concepts/resource-limits-and-performance-optimization#excel-add-ins" You can batch the returned data by using multiple CFs or js api.

wh1t3cAt1k commented 1 year ago

@rundongmsft I believe you implemented automatic payload batching on your end in the related ticket from my original message?

Wondering why it doesn't work in this case?

Unfortunately we cannot use multiple custom functions or the JS API because we need to achieve full reports compatibility with our old XLL add-in, where it works fine.

Logically a call to Velixo's GI() is a single dataset that returns customer-specific data from their accounting system, it's one of our most popular functions and a big selling point for Velixo. It routinely returns dozens of thousands of rows which the customer can then pivot, sort, filter, and aggregate the data in Excel.

It needs to work in exactly the same way across the legacy/new office.js based product, otherwise we simply can't promote migration to the new cross platform solution.

IMO the best solution to this would be to abstract away any payload size limit for CF return values from the add-in developers — and automatically batch those requests in chunks.

cc @keyur32

rundongmsft commented 1 year ago

Hi @wh1t3cAt1k. In your case, CONTOSO.ANSWER() returned around 12.59M data, which will be requested to the server. However, 12.59M exceeds 5M payload size limitation of Excel on the web, so it doesn't work. A feasible idea is let JS API listen to the onChange event. When user inserts the CF in a cell, the CF only saves the user parameters and addresses in the localstorage, not return data. Then JS API will receive the onChange event and execute the callback. In call back, JS API can get the corresponding parameters from the localstorage, batch the request data and set them to the cell where user inserted.

wh1t3cAt1k commented 1 year ago

@rundongmsft apologies if I sound harsh, but this sounds like a terrible kludge, difficult to maintain.

I am also concerned about the performance implications of listening to every possible change across all the cells in the workbook. Furthermore, since this only impacts Excel for the Web, there is a cost of either maintaining two code branches for Web/Non-Web, which defeats the purpose of a cross-platform framework, or resorting to the "ugly" solution for all platforms, even for those that do not have the problem.

I feel pretty strongly that the framework can do a better job at abstracting away any request limits and performing automatic batching of any request payloads with some sort of "long operation happening" feedback on the UI. It feels inhumane to transfer this responsibility to the application developer; the proposed change listener solution is a workaround at best and not a proper fix, and even at that, it's an unusually complex workaround.

As a temporary measure, may I propose to increase the API limit specifically for CF result sets?

At the same time, I do think that automatic batching is the way forward as a general fix.

@JinghuiMS @gmichaud @sshukurov would be grateful for your input here, too.

wh1t3cAt1k commented 1 year ago

As a follow up to the previous comment, another reason why the workaround is not feasible for us because it will overwrite the source formula with the data contents, and also any data that will be in the way of the "spilling range".

As I mentioned, we have a legacy XLL product where this exact formula spills without any trouble, and the other cells depend on this behaviour using references like A9#. If we use macro-like behaviour and onChange dark magic, this will be inconsistent between the products.

We have corporate users who use the XLL-based solution on Windows and their CFOs want to open and refresh the same reports on Mac / Excel Online, which is why the behaviour needs to be 100% consistent.

rundongmsft commented 1 year ago

I do understand that it is important to keep the software consistent across multiple platforms. A better solution to your issue is to let Excel support batching data in single CF requests, which is not currently part of our product. We track Office Add-in feature requests on our Microsoft 365 Developer Platform Ideas Forum. Please add your request there. Feature requests are considered when we go through our planning process.

wh1t3cAt1k commented 1 year ago

@rundongmsft I understand it might be relatively complex to implement API limit batching in CF requests, but we're really in a fix right now because our clients expect to use the same reports on Windows and Mac / Web consistently and it turns out we simply cannot advertise migration to the supposedly "superior" office.js.

Is there a chance that as a temporary measure, the limit be temporarily increased for CF requests to about 20MB specifically until the feature is implemented?

To be fair, there is nothing about that limit applying to the CF results in the documentation, and it's not as if we can batch the result set ourselves in the product - the only thing we can do is return the result set, and it has whatever size the ERP system has returned us. I think it's fair to apply a different kind of limit to the function results by default based on this limitation.

On our end, we will go ahead and submit the feature request on the platform ideas forum.

wh1t3cAt1k commented 1 year ago

I would also like to point out that an issue with exactly the same symptom and very similar reproduction steps: https://github.com/OfficeDev/office-js/issues/2222 was taken into consideration and fixed, but apparently the fix was incomplete?

We already followed up with our clients that #2222 was fixed but apparently there's a different kind of limit now :(

sshukurov commented 1 year ago

I strongly agree with @wh1t3cAt1k that the proposed workaround is absolutely unacceptable as it overwrites the source formula. This is not what users expect and more than that, I'm surprised something like that is being suggested by professionals as a potential measure.

On the other hand, we find the 5MB limitation on a custom function's result size to be completely frustrating, because first of all we find it to be a very strict limit which we can easily exceed (and perhaps not only us?). Otherwise we are also disappointed by the fact it is not documented specifically. I'm familiar with the 5MB payload size limit on a context.sync() call, but I don't find any indication in the Office JS docs about the relevance of that limit for a custom function's result size.

We are so much grateful to the Office JS team for all their efforts on developing and supporting this platform. Our mostly positive experience of it starts to suffer when it comes to this bit, which suggests it's an important shortcoming asking for a prompt action.

rundongmsft commented 1 year ago

Hi @wh1t3cAt1k, here are my answers:

  1. I believe you implemented automatic payload batching on your end in the related ticket from my original message? We implemented automatic payload batching for multiple CFs instead of single CF. A single CF call is the smallest set of requests that cannot be split again. So, when a single CF returns data that exceeds the 5MB payload size limit it still triggers an error.
  2. Is there a chance that as a temporary measure, the limit be temporarily increased for CF requests to about 20MB specifically until the feature is implemented? CF was designed as an API call, we delved into the source code and discussed the solution, we believe that temporarily increasing the limit of CF is equivalent to the difficulty of supporting batching data in single CF request.
  3. I would also like to point out that an issue with exactly the same symptom and very similar reproduction steps: https://github.com/OfficeDev/office-js/issues/2222 was taken into consideration and fixed, but apparently the fix was incomplete? From the repro steps and behavior, I think these two issues are the same issue in Excel Online, which is the data returned by a single CF exceeds the 5MB payload size limit in Excel Online. For the fix #5549594, there was a misunderstanding. Your issue is not related to the fix #5549594. For the real solution (i.e., let Excel support batching data in single CF request) of your issue, I strongly recommend you ask it on Microsoft 365 Developer Platform Ideas Forum. When it gets a lot of votes, we'll implement it earlier.
wh1t3cAt1k commented 1 year ago

Hi @rundongmsft , I have created the developer idea as you requested.

https://techcommunity.microsoft.com/t5/microsoft-365-developer-platform/remove-the-5mb-request-limit-for-custom-function-result-sets-in/idi-p/3629273#M1350

rundongmsft commented 1 year ago

Thanks @wh1t3cAt1k!

wh1t3cAt1k commented 1 year ago

@JinghuiMS @rundongmsft @Rick-Kirkham this has got some attention on the platform, and the status of this item is "under investigation". Can we hope that it gets the product team's attention now?

rundongmsft commented 1 year ago

Hi @wh1t3cAt1k, we have put this new feature requirement in the backlog, but currently our team is working on another higher-priority requirement. We will continue to pay attention and will increase the priority of this new feature when there are more feedback and requirements.

wh1t3cAt1k commented 1 year ago

@Wenjun-Gong @rundongmsft this was discussed on today's Community Call and there were a few other users who mentioned they encountered a similar problem with the request limit.

wh1t3cAt1k commented 1 year ago

Just following up that this issue is critical for our COM -> JS migration strategy, as things are broken for our Excel Online users. Some of our functions return large spilling range datasets from the ERP.