Limit workflow execution query length to 15k characters #2840
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
||
| const queryString = query.toString(); | ||
| if (queryString.length >= MAX_QUERY_LENGTH) { | ||
| query = new URLSearchParams(queryString.substring(0, MAX_QUERY_LENGTH - 1)); |
There was a problem hiding this comment.
Not sure if this is really what we want to be doing here. Seems like the API should maybe just return an error instead. Maybe we can add something via middleware? Open to suggestions!
There was a problem hiding this comment.
Gah ya this is tricky. I don't think we wanna do this because it will almost certainly still result in an error with hacking off the end of the query. I think the spirit of ticket is to prevent the request from ever being sent.
It's really only going to happen on our workflow list query, so I'd suggest we do the check there (onMount with the query param and when triggering via manual query a la copy/paste most likely) and throw error text on the input and prevent triggering the query
There was a problem hiding this comment.
I think it would also happen for the workflow count query, no? Maybe safest to keep something that checks all the requests 🤔
What do you think about a custom response here. That way the error would show up in the UI like this on the /workflows page and the request wouldn't be sent ⬇️
Looks like the max appears on focus |
So it does at height to the input, wondering if we can make the count smaller text |
| {#if maxLength && !disabled && !hideCount} | ||
| <span | ||
| class="invisible text-right text-sm font-medium tracking-widest group-focus-within:visible" | ||
| class="invisible text-right text-xs tracking-widest group-focus-within:visible" |
There was a problem hiding this comment.
So it does at height to the input, wondering if we can make the count smaller text
@Alex-Tideman updated size for both Input and Textarea to be text-xs ⬇️
| const queryIsTooLong = query.toString().length > MAX_QUERY_LENGTH; | ||
| const queryIsTooLong = [...query.values()].some( | ||
| (value) => value.length > MAX_QUERY_LENGTH, | ||
| ); |
There was a problem hiding this comment.
Decided to use values() instead of toString() so the character count should be a little more accurate here. But will still be a little off for the /worklfow-count request because we add a "GROUP BY ExecutionStatus" in addition to whatever is in the query input.
Description & motivation 💭
maxLengthoutside of theInputcomponent and inline with thehintTextbelowSpinnericon with solid versionScreenshots (if applicable) 📸
Design Considerations 🎨
Testing 🧪
How was this tested 👻
Steps for others to test: 🚶🏽♂️🚶🏽♀️
Inputthat hasmaxLengthsetxclear button clears the valuemaxLengthis visible below the input on focus (and ifhideCountisfalse)/workflowspageFilterbutton is disabled if the query in the URL is over 15k charactersChecklists
Draft Checklist
Merge Checklist
Issue(s) closed
DT-3119Docs
Any docs updates needed?