panoply / syncify

🤝 WIP ~ Shopify theme upload, download and watch development tool.
MIT License
132 stars 12 forks source link

Race condition causing 422 Unprocessable Entity error #41

Open WolfGreyDev opened 2 months ago

WolfGreyDev commented 2 months ago

Problem

Sections can contain both .liquid and .json files. The .json files reference the .liquid files as a way to group them together. If a .json file is uploaded before the .liquid file is uploaded, a 422 Unprocessable Entity error is thrown.

Example

header-group.json and header.liquid are both stored in the /sections folder and are uploaded in this order.

header-group.json

header.liquid

```json { "name": "Header Group", "type": "header", "sections": { "header": { "type": "header", "settings": { "color_scheme": "scheme-1" } } }, "order": [ "header" ] } ``` ```liquid

This is the header

{% schema %} { "name": "Header", "class": "section-header", "settings": [] } ```
Failed Success

The Cause

When uploading a theme, Syncify iterates through the files in alphabetical order (presumably). Because header-group.json is uploaded before header.liquid, the referenced section doesn't exist on the store.

If we were to run the upload argument again, it'll work, since the file is now present from the previous upload job.

Solution

Syncify should iterate through all the .liquid files first, then iterate through the .json files.

panoply commented 2 months ago

Which operation should it execute within? (like which command?) or should a check always apply when applying a sync?

WolfGreyDev commented 2 months ago

Dev Build If I were to run syncify store --theme dev --watch --hot, once I save the json file, I'll see this error since the liquid file will not exist. It would be as simple as saving the referenced file and we'll be good again. I don't think it's Syncify's job to handle this here, it's a waste of time.

Production Build Now, if I were to run syncify store --theme dev --upload, I'll run into the problem. So I'd say the --upload flag needs to deal with this.