formio / core

The Form.io Core Javascript Framework
https://formio.github.io/core
MIT License
9 stars 6 forks source link

"tree" property broke submission processing of existing forms on Form.io update #143

Open Susccy opened 2 months ago

Susccy commented 2 months ago

Describe the bug This is more of a request for clarification rather than a bug report. What exactly is the "tree" boolean property on form components supposed to do? In the formio.js Wiki it's only explained very vaguely:

Determines if the Validation should be performed within this component

I don't really understand this description in the context of the word "tree".

This property had been set automatically on many components by the form builder in our existing forms and everything worked as expected so we didn't pay attention to it, but only until we migrated to Form.io v4 (Enterprise v9). With the refactored processing and validation now happening in formio/core, our form submissions suddenly didn't work anymore even though we didn't change the forms at all.

It took us a while to debug and trace the root of the problem in the core source code, eventually noticing that almost all components in the form were considered conditionallyHidden: true in the ProcessContext after being parsed by the process function, even though we made sure their conditionals were met in our test submissions. We tinkered around with the component properties in the form json and eventually realized that removing all occurrences of the "tree" property (no matter if they were true or false) fixed our submissions and they are now being processed properly again (being considered conditionallyHidden: false).

What confuses me even more is that the Wiki documentation on the "tree" property says that it defaults to true when not set, meaning I would expect that true and undefined lead to the same result. However our submissions only work when I fully remove the property and leaving it on true breaks the submissions, so there's obviously a difference between true and undefined.

Version/Branch formio v4.0.2 @formio/core v2.0.1

travist commented 2 months ago

I appreciate your detailed report and we will certainly bring this up with our internal meetings to have it investigated.

The history of the "tree" property really stems from our need to enable "custom" nested data components the ability to signal the server that they are a "nested" data structure, such as you would see with DataGrid and EditGrid components. Considering that you are unable to execute (at least not yet) your custom validators on the server, the validator needed a way to know if it should iterate into the component tree to also include the structure of your nested components as part of the validation.

The fact that this broke with 9.x server was definitely not an intended change. We did have discussions on the ability to "deprecate" this property but we never make decisions where we would knowingly break reverse compatibility. Regardless, we will look into it.

lane-formio commented 2 months ago

For internal reference: FIO-8957

Susccy commented 1 month ago

Update from our side: as we were removing the tree property from more of our forms, we eventually came to a form where removing it lead to a new error - posting a submission for that form causes the form.io server to respond with a 400 Bad Request: "Script execution timed out."

Will need to dig into formio/core again to find the cause for this...

brendanbond commented 1 month ago

Hi @Susccy thanks for the update.

  1. Are you using a self-hosted version of our enterprise software or perhaps our SaaS offering? If so, you might consider contacting support@form.io so we can better assist you. We're happy to keep this on GitHub, but we're a very small team and give much more responsive support to our Enterprise customers; if you are one, feel free to utilize that channel!
  2. Although we still want to dig into the original issue, and it very well could be that removing the tree property from your forms has caused some unintended performance regressions, "Script execution timed out." has to do with the way that versions >= 9.2 and above of the Enterprise Server (and I believe versions >= 4.2 of the open source server) sandbox untrusted JavaScript code (e.g. in your forms). Due to some security issues in an underlying dependency we previously used to sandbox JavaScript execution on the server, we had to completely revamp how that code is sandboxed and introduced a timeout variable so arbitrary scripts couldn't hang the server (e.g. while (true) {}) or introduce memory leaks, etc. etc. Depending on the version of the software you're running (if you're self-hosted), this is configurable in the Enterprise Server via the FORMIO_VM_TIMEOUT environment variable.
Susccy commented 1 month ago

Thank you for the quick reply @brendanbond ! As stated in my original post above, we are currently on formio v4.0.2 which internally uses formio/core v2.0.1 and formio/vm v0.0.8. Is it possible that the timeout error you described happens on this version too, not only on >=4.2 ?

We are only using the open source offerings by Form.io for our apps (formio API and formiojs), no Enterprise package.

brendanbond commented 1 month ago

Yeah I've almost certainly got the versions wrong there, it might be 9.00 / 4.00 and above. The error you're seeing is in the VM, which for now unfortunately is a private repository and the timeout may not be configurable in that version. Let me look into 4.0.2 and see if there's something we can do here for that particular error

Susccy commented 1 month ago

Thank you very much for looking into this! In the meantime we will look internally if we can give more hardware resources to the formio server so the vm execution can finish faster.

brendanbond commented 1 month ago

@Susccy it looks like this was added in version 4.2.0 of formio

Susccy commented 1 month ago

@brendanbond so that means before 4.2 the same timeout error occurs and is not configurable?

in that case our only options would be to either update to the latest version or provide sufficient resources to the server so the execution finishes quicker?

brendanbond commented 3 weeks ago

@Susccy Yes