search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.6k stars 8.21k forks source link

[Expressions] Ambiguous error for missing args #189770

Open nickofthyme opened 3 months ago

nickofthyme commented 3 months ago

I came across a scenario the other day trying to add a new prop to an expression function. The example was adding a colorMapping prop to the args of the lens_datatable_column expression below.

https://github.com/elastic/kibana/blob/e13a1baf72bb8250375aaa52a371cb6f34d99d53/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts#L59-L88

Adding this new prop in all places had proper type checking with the exception of the args defined as...

https://github.com/elastic/kibana/blob/e13a1baf72bb8250375aaa52a371cb6f34d99d53/src/plugins/expressions/common/expression_functions/types.ts#L77

This resulted in a dynamic error that is not forwarded to the console or UI is a meaningful way, only showing the error message below...

Screenshot 2024-08-01 at 5 15 09 PM
Error log errors ``` Warning: Cannot update a component (`VisualizationWrapper`) while rendering a different component (`ReactExpressionRenderer`). To locate the bad setState() call inside `ReactExpressionRenderer`, follow the stack trace as described in https://reactjs.org/link/setstate-in-render at ReactExpressionRenderer (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/expressions/1.0.0/expressions.plugin.js:9885:3) at Suspense at ReactExpressionRenderer at div at VisualizationWrapper (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:13252:3) at div at SingleDropInner (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:1884:3) at div at DroppableImpl (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:1699:5) at Droppable (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:1677:104) at div at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at EuiFlexItemInternal (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:84711:23) at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at div at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at EuiFlexGroupInternal (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:84485:24) at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at div at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at section at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at EuiPageSection (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:111628:23) at _EuiPageSection (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:112521:78) at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at main at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at _EuiPageInner (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:112140:23) at div at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at _EuiPageOuter (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:112275:23) at _EuiPageTemplate (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:112418:23) at WorkspacePanelWrapper (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:13526:3) at InnerWorkspacePanel (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:12867:3) at WorkspacePanel (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:12853:5) at ErrorBoundary (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:13783:5) at section at div at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at EuiPageBody (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:110962:23) at div at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at EuiPage (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:110816:23) at div at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at EuiFlexItemInternal (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:84711:23) at div at http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150023:73 at EuiFlexGroupInternal (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:84485:24) at FrameLayout (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:11178:97) at ChildDragDropProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:3167:3) at RootDragDropProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:3094:3) at EditorFrame (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.2.js:10879:5) at div at EditorFrameContainer (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.5.js:54552:11) at EditorFrameWrapper (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.5.js:27656:3) at div at App (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.5.js:27233:3) at Provider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:370581:20) at http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.5.js:28878:97 at EditorRoute (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.5.js:28984:22) at Route (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:377332:29) at Route (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.5.js:26411:3) at Switch (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:377534:29) at Routes (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.5.js:26543:3) at RenderedRoute (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:375499:5) at Routes (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:376060:5) at Router (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:375998:15) at CompatRouter (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:374025:5) at Router (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:376951:30) at HashRouter (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:376424:35) at HashRouter (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.chunk.5.js:26488:3) at http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/presentationUtil/1.0.0/presentationUtil.plugin.js:3640:7 at http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/presentationUtil/1.0.0/presentationUtil.plugin.js:3640:7 at http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/presentationUtil/1.0.0/presentationUtil.plugin.js:3640:7 at http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/presentationUtil/1.0.0/presentationUtil.plugin.js:3640:7 at http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/presentationUtil/1.0.0/presentationUtil.plugin.js:3640:7 at provider (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/presentationUtil/1.0.0/presentationUtil.plugin.js:3833:7) at Provider (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/kibanaReact/1.0.0/kibanaReact.plugin.js:2886:15) at ErrorBoundaryInternal (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:45754:5) at KibanaErrorBoundary (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:45812:110) at KibanaErrorBoundaryProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:45577:3) at EuiContext (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:63085:24) at IntlProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:219237:47) at I18nProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:41192:3) at I18nContext (http://localhost:5620/XXXXXXXXXXXX/bundles/core/core.entry.js:19742:9) at EuiComponentDefaultsProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:115622:36) at CurrentEuiBreakpointProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:140442:23) at ThemeProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:150086:63) at EuiEmotionThemeProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:145439:23) at EuiThemeMemoizedStylesProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:145885:23) at EuiThemeProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:145702:22) at EuiCacheProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:115543:20) at EuiProviderNestedCheck (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:115775:23) at EuiProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:115833:25) at KibanaEuiProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.plugin.js:2584:10) at KibanaRootContextProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.plugin.js:2692:3) at KibanaRenderContextProvider (http://localhost:5620/XXXXXXXXXXXX/bundles/plugin/lens/1.0.0/lens.plugin.js:2496:3) ```

I only managed to get a sensible error when I paused execution when setDynamicError was called from here...

https://github.com/elastic/kibana/blob/e13a1baf72bb8250375aaa52a371cb6f34d99d53/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx#L678

This finally told me the real problem...

[lens_datatable] > [lens_datatable_column] > Unknown argument 'colorMapping' passed to function 'lens_datatable_column'

From this I think we should enforce all args to have an ArgumentType listed in for the function.

Additionally, we could consider logging the dynamic errors to the console, potentially only in development.

elasticmachine commented 3 months ago

Pinging @elastic/kibana-visualizations (Team:Visualizations)

mbondyra commented 3 months ago

Hi Nick,

Additionally, we could consider logging the dynamic errors to the console, potentially only in development.

I think there's no harm in logging them in any environment - this way we can debug easier when helping customers.

From this I think we should enforce all args to have an ArgumentType listed in for the function.

this is not clear for me, what do you mean?

nickofthyme commented 3 months ago

Sounds good to me! You actually fixed the main issue last week with https://github.com/elastic/kibana/pull/189161 where the longMessage here was empty and even though it was displaying the WorkspaceErrors it was not showing anything useful message. I think that is sufficient rather than logging the error to the console.

As for the other point to enforce all types, I opened up https://github.com/elastic/kibana/pull/189772 that simply loops over all the keys in the provided Arguments type but makes them required. So this is to ensure all arguments have a corresponding ArgumentType defined regardless if the arg is optional.

https://github.com/elastic/kibana/blob/97ed056b278419e8531afe825f963d9c7b9eb96d/src/plugins/expressions/common/expression_functions/arguments.ts#L11-L20

Maybe you know of a case where some of the arguments are not needed to be added to the args definitions. But from my perspective it makes sense for all args to have a corresponding ArgumentType definition, especially if not listing them makes it throw a runtime error.

Currently the types only enforce the non-optional arguments. In the example below the only required arg is columnId, if I add an ArgumentType for just columnId the types are content, then later if I use the function with any of the other listed args, boom 💥 .

image