Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Interpreter clobbers browser functions with server functions #33039

Closed
w33ble opened this issue Mar 12, 2019 · 4 comments · Fixed by #57196
Closed

Interpreter clobbers browser functions with server functions #33039

w33ble opened this issue Mar 12, 2019 · 4 comments · Fixed by #57196
Assignees
@ppisljar
Labels
bug Fixes for quality problems that affect the customer experience Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline)

Comments

@w33ble
Copy link
Contributor

w33ble commented Mar 12, 2019 •
edited
Loading

Kibana version: 6.7+

Describe the bug:

With the move away from websockets, and moving the interpreter to the browser, there was code introduced that changes how server functions work, which creates a copy of the function for use in the browser that actually sends the work to the server.

The problem is that if you have the same function defined in both registries, the browser one gets clobbered and that function will always just run on the server instead.

The to function in Canvas is a great example of this. It's a common function which Canvas registers in both browser and server function registries, but it only ever gets run on the server because the browser definition is overwritten.

Steps to reproduce:

  1. Use the to function (prior to Fix: correctly access types from "to" function #32691)

  2. Alternatively, register a function (new or existing) in both registries and console.log the value of handlers.environment, it will always say "server"
@w33ble w33ble added bug Fixes for quality problems that affect the customer experience Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Mar 12, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@w33ble w33ble mentioned this issue Mar 12, 2019
Merged
w33ble added a commit to w33ble/kibana that referenced this issue Mar 12, 2019
@w33ble
chore: move to function to browser
213d510 
types can be accessed in the browser now, and a common function makes no sense right now. also elastic#33039 prevents having the same function in both places anyway
w33ble added a commit to w33ble/kibana that referenced this issue Mar 12, 2019
@w33ble
Fix: correctly access types from "to" function (elastic#32691)
8170951 
* fix: correctly access types

they are not exposed on the handlers object, they must be imported from @kbn/interpreter

* chore: move to function to browser

types can be accessed in the browser now, and a common function makes no sense right now. also elastic#33039 prevents having the same function in both places anyway
w33ble added a commit that referenced this issue Mar 12, 2019
@w33ble
Fix: correctly access types from "to" function (#32691)
7c7bcdd 
* fix: correctly access types

they are not exposed on the handlers object, they must be imported from @kbn/interpreter

* chore: move to function to browser

types can be accessed in the browser now, and a common function makes no sense right now. also #33039 prevents having the same function in both places anyway
w33ble added a commit to w33ble/kibana that referenced this issue Mar 12, 2019
@w33ble
Fix: correctly access types from "to" function (elastic#32691)
406f862 
* fix: correctly access types

they are not exposed on the handlers object, they must be imported from @kbn/interpreter

* chore: move to function to browser

types can be accessed in the browser now, and a common function makes no sense right now. also elastic#33039 prevents having the same function in both places anyway
w33ble added a commit to w33ble/kibana that referenced this issue Mar 12, 2019
@w33ble
Fix: correctly access types from "to" function (elastic#32691)
da114e4 
* fix: correctly access types

they are not exposed on the handlers object, they must be imported from @kbn/interpreter

* chore: move to function to browser

types can be accessed in the browser now, and a common function makes no sense right now. also elastic#33039 prevents having the same function in both places anyway
w33ble added a commit that referenced this issue Mar 12, 2019
@w33ble
Fix: correctly access types from "to" function (#32691) (#33068)
2aa8f62 
* fix: correctly access types

they are not exposed on the handlers object, they must be imported from @kbn/interpreter

* chore: move to function to browser

types can be accessed in the browser now, and a common function makes no sense right now. also #33039 prevents having the same function in both places anyway
w33ble added a commit that referenced this issue Mar 12, 2019
@w33ble
Fix: correctly access types from "to" function (#32691) (#33066)
a15b172 
* fix: correctly access types

they are not exposed on the handlers object, they must be imported from @kbn/interpreter

* chore: move to function to browser

types can be accessed in the browser now, and a common function makes no sense right now. also #33039 prevents having the same function in both places anyway
w33ble added a commit that referenced this issue Mar 12, 2019
@w33ble
Fix: correctly access types from "to" function (#32691) (#33067)
9176f96 
* fix: correctly access types

they are not exposed on the handlers object, they must be imported from @kbn/interpreter

* chore: move to function to browser

types can be accessed in the browser now, and a common function makes no sense right now. also #33039 prevents having the same function in both places anyway
@ppisljar ppisljar added :AppArch and removed Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Mar 27, 2019
@ppisljar
Copy link
Member

@w33ble can this be closed ?

@w33ble
Copy link
Contributor Author

w33ble commented Mar 27, 2019

@ppisljar It hasn't been fixed as far as I know. Are you trying to make the case that it's not actually a problem?

@ppisljar
Copy link
Member

no i was just wondering if its fixed

@lukeelmers lukeelmers added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label Apr 5, 2019
@lukeelmers lukeelmers assigned lukeelmers and ppisljar and unassigned lukeelmers Jan 30, 2020
@ppisljar ppisljar mentioned this issue Feb 10, 2020
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ppisljar ppisljar

Labels
bug Fixes for quality problems that affect the customer experience Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

don't register a wrapper if browser side function exists. ppisljar/kibana
4 participants
@w33ble @lukeelmers @ppisljar @elasticmachine