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
641 stars 92 forks source link

[BLOCKER] Volume licensed Office 2021 (LTSC version) crashes with add-in which uses Custom Data Types for Custom functions #4385

Open 4tti opened 3 weeks ago

4tti commented 3 weeks ago

Provide required information needed to triage your issue

When the Add-in is using custom functions with Custom Data type support (i.e. having allowCustomDataForDataTypeAny as described in https://learn.microsoft.com/en-us/office/dev/add-ins/excel/custom-functions-json#allowcustomdatafordatatypeany) the Office 2021 LTSC will crash whenever such custom function is used (e.g. you have repeating argument of type any).

Your Environment

Expected behavior

Excel should not crash. The custom function should rather return #VALUE! error or even better would be to have fallback that will ignore allowCustomDataForDataTypeAny.

Current behavior

Excel crashes.

Steps to reproduce

  1. Create simple addin with custom function that has repeating argument of type Any
  2. Make sure the addin had custom functions enabled with allowCustomDataForDataTypeAny
  3. Open Excel (LTSC) and insert the custom function into the sheet

Provide additional details

Dump shows this exception: Unhandled exception at 0x00007FFD862989C2 (mso20win32client.dll) in EXCEL.EXE_240417_095617.dmp: Unknown __fastfail() status code: 0x0000000000000000.

Context

This is show stopper as we introduced 'preview' custom functions that were meant to work with proper clients but there was expectation that the non-support clients will not crash. We are getting more and more feedback how it affects our customers.

MiaofeiWang commented 3 weeks ago

Hi @4tti , I am not able to reproduce the crash issue with a simple function. My Excel build is 14332.20685 and here are the function JS and metadata.

/**
 * @customfunction
 * @param {any[]} input Input grouped parameters
 * @returns {number} return result
 */
function EchoGroupedParameter(input) {
  return 1;
}
{
    "allowCustomDataForDataTypeAny": true,
    "functions": [
        {
            "id": "ECHOGROUPEDPARAMETER",
            "name": "ECHOGROUPEDPARAMETER",
            "parameters": [
                {
                    "description": "Input grouped parameters",
                    "name": "input",
                    "repeating": true,
                    "type": "any"
                }
            ],
            "result": {
                "type": "number"
            }
        },
    ]
}

Please let me know if I missed any step to reproduce. And I am wondering if it is because of any JS code logic inside of the function.

4tti commented 3 weeks ago

Hello @MiaofeiWang,

Thanks a lot! Let us check and I'll get back to you.

4tti commented 2 weeks ago

Sorry, did not have time so far, will get back to you by end of the week hopefully.

4tti commented 2 weeks ago

@MiaofeiWang can you please modify the code to have "dimensionality": "matrix" and e.g. the formula can return sum of all the numbers within all referenced ranges. Then to repro please add '1' to A1:F5 and then in G1 add formula like '=EchoGroupedParameter(A1:A4,B1:B3,C1:D5,E1:F4)'

Does it work on your end?

PS: also set these options: "options": { "volatile": false, "requiresAddress": false, "cancelable": true, "stream": false } And also include "allowErrorForDataTypeAny": true

MiaofeiWang commented 1 week ago

@4tti I am still not able to repro this issue on build 14332.20685. Can you share a minimal repro code inside of your function?

image

Metadata:

        {
            "description": "Sum of all numbers",
            "id": "SUMALL",
            "name": "SUMALL",
            "options": {
                "volatile": false,
                "requiresAddress": false,
                "cancelable": true,
                "stream": false
            },
            "parameters": [
                {
                    "dimensionality": "matrix",
                    "name": "operands",
                    "repeating": true,
                    "type": "any"
                }
            ],
            "result": {
                "type": "number"
            }
        },

Function JS:

/**
 * Sum of all numbers
 * @customfunction
 * @param {any[][][]} operands
 * @returns {number} The sum of all the numbers in the operands.
 */
function SumAll(operands) {
  let total = 0;
  operands.forEach((range) => {
    range.forEach((row) => {
      row.forEach((cell) => {
        total += cell;
      });
    });
  });

  return total;
}
4tti commented 1 week ago

Hello @MiaofeiWang And did you include

"allowCustomDataForDataTypeAny": true,
"allowErrorForDataTypeAny": true

in the metadata?

Our metadata:

{
    "$schema": "https://developer.microsoft.com/en-us/json-schemas/office-js/custom-functions.schema.json",
    "allowCustomDataForDataTypeAny": true,
    "allowErrorForDataTypeAny": true,
    "functions": [
        {
            "id": "INFOR_BP_OIS.JoinArrays",
            "name": "INFOR.JOINARRAYS",
            "description": "(Preview version) Returns matrix of values of 'any' type \"joined\" by columns.",
            "helpUrl": "http://www.infor.com",
            "result": {
                "type": "any",
                "dimensionality": "matrix"
            },
            "parameters": [
                {
                    "name": "use_crossjoin",
                    "description": "Boolean value for type of 'join' to be used. False for simple merge or true for full cross-join (all combinations). Default value: false.",
                    "optional": false,
                    "repeating": false,
                    "type": "boolean",
                    "dimensionality": "scalar"
                },
                {
                    "name": "first_array",
                    "description": "Matrix containing cell-reference or array argument.",
                    "optional": false,
                    "repeating": false,
                    "type": "any",
                    "dimensionality": "matrix"
                },
                {
                    "name": "second_array",
                    "description": "Matrix containing cell-reference or array argument.",
                    "optional": false,
                    "repeating": false,
                    "type": "any",
                    "dimensionality": "matrix"
                },
                {
                    "name": "array",
                    "description": "One or more matrices containing cell-reference or array argument.",
                    "optional": true,
                    "repeating": true,
                    "type": "any",
                    "dimensionality": "matrix"
                }
            ],
            "options": {
                "volatile": false,
                "requiresAddress": false,
                "cancelable": true,
                "stream": false
            }
        }
    ]
}

our typescript code for that function:

const unionMatrixes = (matrixes: any[]): any[][] => {
    let rowsCount: number = 0;
    let columnsCount: number = 0;

    const matrixesCount: number = matrixes.length;

    const matrixColumnCount: number[] = [];
    const matrixRowCount: number[] = [];

    for (let i = 0; i < matrixesCount; ++i) {
        matrixColumnCount[i] = ArrayHelpers.getColumnsCountFromJaggedArray(matrixes[i]);
        columnsCount += matrixColumnCount[i];
        matrixRowCount[i] = matrixes[i].length;
        rowsCount = Math.max(rowsCount, matrixRowCount[i]);
    }

    const unionArray: any[][] = new Array(rowsCount);

    for (let i = 0; i < rowsCount; ++i) {
        unionArray[i] = new Array(columnsCount);
    }

    let columnIdxStart: number = 0;

    for (let i = 0; i < matrixesCount; ++i) {
        let destRowIdx: number = 0;

        for (let srcRowIdx = 0; srcRowIdx < matrixRowCount[i]; ++srcRowIdx) {
            let destColumnIdx: number = columnIdxStart;

            for (let srcColumnIdx = 0; srcColumnIdx < matrixColumnCount[i]; ++srcColumnIdx) {
                unionArray[destRowIdx][destColumnIdx] = matrixes[i][srcRowIdx][srcColumnIdx];
                ++destColumnIdx;
            }
            ++destRowIdx;
        }
        columnIdxStart += matrixColumnCount[i];
    }

    return unionArray;
};

const concatArrays = (first: any[][], second: any[][], arrayParams: any[][] | null | undefined): any[][] => {
    const firstArray: any[][] = [first, second];
    if (arrayParams == null) {
        return firstArray;
    }

    return firstArray.concat(arrayParams);
};

const createCrossJoinMatrix = (matrixA: any[][], matrixB: any[][]): any[][] => {

    const matrixARowsCount: number = matrixA.length;
    const matrixBRowsCount: number = matrixB.length;

    const totalRowsCount: number = matrixARowsCount * matrixBRowsCount;

    const matrixAColumnsCount: number = ArrayHelpers.getColumnsCount(matrixA);
    const matrixBColumnsCount: number = ArrayHelpers.getColumnsCount(matrixB);
    const totalColumnsCount: number = matrixAColumnsCount + matrixBColumnsCount;

    const isAEmpty: boolean = (matrixARowsCount * matrixAColumnsCount) === 0;
    const isBEmpty: boolean = (matrixBRowsCount * matrixBColumnsCount) === 0;

    if (isAEmpty || isBEmpty) {
        return new Array(0);
    }

    const crossjoin: any[][] = new Array(totalRowsCount);

    for (let i = 0; i < totalRowsCount; ++i) {
        crossjoin[i] = new Array(totalColumnsCount);
    }

    let rowIdx: number = 0;
    for (let rowAIdx = 0; rowAIdx < matrixARowsCount; ++rowAIdx) {

        let columnIdx: number = 0;
        for (let columnAIdx = 0; columnAIdx < matrixAColumnsCount; ++columnAIdx) {

            for (let idxSubRowB = 0; idxSubRowB < matrixBRowsCount; ++idxSubRowB) {
                crossjoin[rowIdx + idxSubRowB][columnIdx] = matrixA[rowAIdx][columnAIdx];
            }
            ++columnIdx;
        }
        rowIdx += matrixBRowsCount;
    }

    let rowIdxStart: number = 0;
    for (let rowBIdx = 0; rowBIdx < matrixBRowsCount; ++rowBIdx) {
        let columnIdx: number = matrixAColumnsCount;

        for (let columnBIdx = 0; columnBIdx < matrixBColumnsCount; ++columnBIdx) {
            rowIdx = rowIdxStart;

            for (let idxSubRowA = 0; idxSubRowA < matrixARowsCount; ++idxSubRowA) {
                crossjoin[rowIdx][columnIdx] = matrixB[rowBIdx][columnBIdx];
                rowIdx += matrixBRowsCount;
            }
            ++columnIdx;
        }
        ++rowIdxStart;
    }

    return crossjoin;
};

export async function joinArrays(crossjoin: boolean, firstArray: any[][], secondArray: any[][], array: any[][] | null | undefined, _invocation: CustomFunctions.CancelableInvocation) {
    if (!crossjoin) {
        return unionMatrixes(concatArrays(firstArray, secondArray, array));
    }

    let tmpCrossJoin: any[][] = createCrossJoinMatrix(firstArray, secondArray);
    if (array == null) {
        return tmpCrossJoin;
    }

    const matrixCount: number = array.length;

    for (let i = 0; i < matrixCount; ++i) {
        tmpCrossJoin = createCrossJoinMatrix(tmpCrossJoin, array[i]);
    }

    return tmpCrossJoin;
}
MiaofeiWang commented 1 week ago

@4tti I am able to reproduce the crash on Office 2021 if including both "allowCustomDataForDataTypeAny": true, and "allowErrorForDataTypeAny": true. At the same time, I double confirmed that it will not crash if not including "allowErrorForDataTypeAny": true in metadata, which is why I was not able to reproduce the issue before.

As a solution for your add-in, could you try to remove the "allowErrorForDataTypeAny": true from your metadata and check if it can fix the crash? Regarding functionalities, with or without this doesn't make difference as long as you have "allowCustomDataForDataTypeAny": true which can cover both rich data types and rich errors.

For the crash fix, we have created internal work item 8927383 to track.

4tti commented 1 week ago

Thanks a lot, we already have the workaround in place!