Developer-Mike / obsidian-advanced-canvas

⚡ Supercharge your Obsidian.md canvas experience! Create presentations, flowcharts and more!
GNU General Public License v3.0
286 stars 12 forks source link

[BUG] Unable To Open if Plugin Is Disabled and Canvas Interacted With #82

Closed itsonlyjames closed 3 months ago

itsonlyjames commented 3 months ago

Steps to Reproduce

! Only happens when a canvas has a portal.

Steps to repro:

  1. Create canvas, interact with it while Advanced Canvas is enabled and in use
  2. Close file, disable Advanced Canvas
  3. Reopen canvas, and interact with canvas (move object, colour change, etc.)
  4. Reenable Advanced Canvas and try to open canvas
  5. Receive "failed to open" message

image

This bug was discovered as I was testing whether it was your plugin or Obsidian native that updates interaction path colours on colour change (it's Obsidian). Encountered this bug—bringing it to your attention. It's likely some mismatch with a particular JSON structure.

I'm eager to get help with fixing this, because this could be detrimental to the ongoing usage of canvas'. I'd hate someone to have made a lot of progress on a canvas, encounter this issue, and not have version control to save them like I'm fortunate enough to have.

Code to Reproduce

  1. Create new canvas
  2. Edit canvas in text editor and add the below snippet
  3. Try to open canvas in Obsidian (even if Advanced Canvas has been enabled again issue still exists) Code example that causes error:
    {
    "nodes":[
    {
      "id":"22804ef39aa7c3af",
      "type":"text",
      "text":"Test2",
      "styleAttributes":{},
      "x":-180,
      "y":40,
      "width":260,
      "height":60,
      "color":"5"
    },
    {
      "id":"1d7fbb2f35817b01",
      "type":"text",
      "text":"Test",
      "styleAttributes":{},
      "x":-180,
      "y":-120,
      "width":260,
      "height":60,
      "color":"4"
    },
    {
      "id":"83a862265cebf02e",
      "type":"text",
      "text":"Cool",
      "styleAttributes":{},
      "x":-420,
      "y":-280,
      "width":260,
      "height":60,
      "color":"5"
    },
    {
      "id":"73eb311b22336d56",
      "type":"file",
      "file":"Untitled 1.canvas",
      "portalToFile":"Untitled 1.canvas",
      "portalIdMaps":{
        "nodeIdMap":{"73eb311b22336d56-9331e0874bada340":"9331e0874bada340"},
        "edgeIdMap":{}
      },
      "closedPortalWidth":400,
      "closedPortalHeight":400,
      "x":-880,
      "y":-40,
      "width":360,
      "height":200,
    },
    {
      "id":"73eb311b22336d56-9331e0874bada340",
      "type":"text",
      "text":"Cool",
      "styleAttributes":{},
      "portalId":"73eb311b22336d56",
      "x":-830,
      "y":10,
      "width":260,
      "height":60
    }
    ],
    "edges":[
    {
      "id":"32e4f60a9f8a5cfa",
      "styleAttributes":{},
      "fromNode":"1d7fbb2f35817b01",
      "fromSide":"bottom",
      "toNode":"22804ef39aa7c3af",
      "toSide":"top",
      "color":"4"
    }
    ],
    "metadata":{}
    }
Developer-Mike commented 3 months ago

"height":200,

This trailing comma is the cause for it. I already encountered this bug while writing the Advanced Canvas plugin. I fixed it by using the default JSON.stringify function (Here is the fix (L43 excluded)).

This behavior is definitely a bug in the official Canvas. I suspect that the custom properties (added by Advanced Canvas) of the nodes cause this. Specifically, the custom properties with the type of an Object.

Now that this issue surfaced again, I'll try to report the bug to the official obsidian team (Idk how yet 😅). The problem is that the plugin can't fix the vanilla behavior due to it being disabled. Additionally, I'll try to add a script to fix the canvas file again when opened with the Advanced Canvas plugin enabled.

Developer-Mike commented 3 months ago

So I managed to narrow the issue down. Here is an MRE

{
    "nodes":[
        {
            "id":"73eb311b22336d56",
            "type":"file",
            "file":"Any File.md",
            "customProperty":{},
            "x":-880,
            "y":-40,
            "width":360,
            "height":220
        },
        {"id":"22804ef39aa7c3af","type":"text","text":"Modify this canvas and try to reopen it...\n\nIt adds a \",\" after the height property of the **file** node. Rendering the JSON invalid.\n\nThis only happens if a custom property of type \"Object\" (`{}`) or \"List\" (`[]`) is defined.\n\nIt doesn't matter if the linked file exists or not.","x":-880,"y":-380,"width":360,"height":300,"color":"5"}
    ],
    "edges":[]
}

The wrong save is caused by the customProperty: {} (type has to be an object or list) inside the file node.

itsonlyjames commented 3 months ago

@Developer-Mike Are you sure it's the customProperty: {}? I removed that from the canvas, and when it updates it's adding the trailing comma to height: x,, or even color: ..., if that's the last property. Seemingly, whatever the last property is within a file node is having a trailing comma appended.

Update the below with Advanced Canvas disabled and you'll notice it adds a trailing comma to colour: ..., and if you remove color, it will add it to height: ...,.

MRE created with Advanced Canvas enabled, then disabling and altering (causes bug).

{
    "nodes":[
        {
            "id":"73eb311b22336d56",
            "type":"file",
            "file":"Any File.md",
            "styleAttributes":{},
            "x":-880,
            "y":-40,
            "width":360,
            "height":220,
            "color":"6"
        },
        {
            "id":"22804ef39aa7c3af",
            "type":"text",
            "text":"Modify this canvas and try to reopen it...\n\nIt adds a \",\" after the height property of the **file** node. Rendering the JSON invalid.\n\nThis only happens if a custom property of type \"Object\" (`{}`) or \"List\" (`[]`) is defined.\n\nIt doesn't matter if the linked file exists or not.",
            "styleAttributes":{},
            "x":-880,
            "y":-440,
            "width":360,
            "height":300,
            "color":"5"
        }
    ],
    "edges":[]
}

MRE created with Advanced Canvas disabled, so native Obsdian Canvas (this does not break, no matter the alterations made):

{
    "nodes":[
        {"id":"9496b3657781a5cd","type":"file","file":"any file.canvas","x":-260,"y":-320,"width":400,"height":400},
        {"id":"943bef195c1e3735","type":"text","file":"file.md","text":"Test","x":-40,"y":-400,"width":250,"height":60}
    ],
    "edges":[]
}
Developer-Mike commented 3 months ago

Even in your example, the core problem stays the same. As soon as a file node has a custom property of type object or list (in your example styleAttributes), the file will get corrupted due to a trailing comma.

itsonlyjames commented 3 months ago

@Developer-Mike ahh yep, I've tested and see this now. It is also the only differentiating factor between the two provided snippets. I saw your bug report in the Obsidian forums. Happy for you to direct this ticket, should you wish to close it.

Side note, this is an incredible plugin! Your work is greatly appreciated, and absolutely enhances the canvas experience. Thank you 👏

Developer-Mike commented 3 months ago

Just for having a link to the bug report here: https://forum.obsidian.md/t/canvas-file-corruption-if-file-node-has-custom-property-of-type-object-or-list/84698

TomBirkland commented 3 months ago

Sorry to jump in with a noob question, but I am not a coder so I am a bit at a loss here. Does this mean that the bug is in the Obsidian core application, and that this bug will not be remedied unless or until Obsidian is repaired? Or does this mean that an update of this plugin is forthcoming? I ask because I really would like to use this Advanced Canvas plugin! But it appears that I cannot really use it until the bug is remedied somewhere. Of course, if you have a snippet of code I could just insert into the text file as a workaround, I am comfortable trying that. Sorry if this question is inappropriate for this venue.

Developer-Mike commented 3 months ago

Hey @TomBirkland no need to apologize. Yes, this bug is in the core canvas feature maintained by the Obsidian team.

BUT it only occurs if all the following criteria are met:

  1. Your canvas does contain a file node
  2. You've opened the canvas at least once with the Advanced Canvas plugin enabled
  3. You open the canvas again, this time with the Advanced Canvas plugin disabled
  4. You modify and save the file Your canvas is now "broken"

Keep in mind that a "broken" file can easily be fixed again (No data loss possible). And if you don't plan to disable the Advanced Canvas plugin, you're completely fine.

In the next version of Advanced Canvas, there will be a feature to open the "broken" file as normally. The one disadvantage is, that the ability to open the "broken" file only works when Advanced Canvas is enabled.

TomBirkland commented 3 months ago

Thanks, Mike. This is very helpful. Some quick questions and then I will let this rest. But first, a note of appreciation for your work and for this plug-in! I really appreciate it.

  1. What is a file node? Is there a way to avoid creating one in a Canvas?
  2. I think I understand the problem with the appended commas, but you say that the file is easily fixed. Can you tell me how I would do that? I have been able to find old file snapshots and restore those, but these do seem to yield some data loss.
  3. It seems that this problem can be avoided as long as I always open the file with Advanced Canvas installed and enabled. Is that right? I think I am introducing the error when I open the file on a machine that does not yet have the plug-in installed, such as on my iPad or a different computer. In other words, it sounds like I can avoid this error as long as I have Advanced Canvas installed and enabled on all machines all the time. If that's the case, that's easily remedied!

Thanks again. If nothing else, I am learning that I could do well to learn more about JSON syntax!

Developer-Mike commented 3 months ago

@TomBirkland thank you for your feedback on the plugin. And don't worry about asking too much.

  1. A file node is one of four node types. There are text, file, link and group nodes. It is this rectangle where you can add another note to the canvas. To avoid it, you can just stop using it (it's pretty useful, so I don't suggest it)
  2. In JSON (the file format a canvas file is saved), individual key-value pairs must be separated using commas. The problem in the core obsidian is, that, if this bug occurs, a key-value pair has a trailing comma without any key-value pair following it. This causes a read error if the canvas file gets opened again. To restore it, you don't have to load any previous snapshot of the file. Just open the .canvas file in a text editor, search the trailing comma, delete only the comma and save the file. Info: Searching this comma can be a bit tricky. Because of this, the next Advanced Canvas update will enable you to open those files like others.
  3. Yes! If the file always gets opened with the plugin Enabled, this issue will never occur.
TomBirkland commented 3 months ago

Thanks, Mike. This is very helpful, and I will be checking for the next update. I appreciate your quick replies!

itsonlyjames commented 3 months ago

I investigated a potential fix for this, because I was also fearful of having an issue arise and having to undo an astronominal amount of work on a canvas and then have this issue arrise. Now of course the issue only exists if you disable Advanced Canvas, change the canvas, and reenable it, but in the interest of assisting others who may face this, I created this regex pattern to assist in finding the culprit trailing comma—meaning you don't have to manually search hundreds if not thousdands of lines of code. This will super helpful to fix the issue, rather than having to rely on version control and potentially losing a bunch of hard work.

Regex: ,\n^((?:\d+\.)? +)\}|,}

This works for both multi line inputs as well as single line inputs.

See screenshots below:

image image

Then if you wanted to bulk fix the issue you could so like so (note the indent which is required on the closing bracket):

image
Developer-Mike commented 3 months ago

Thank you for sharing your possible fix. The one problem with a regex is that it will also replace matching text content. Although it's very unlikely to have a match, the risk is still too high for it being added to the plugin. For manual fixing it's perfect tho.

itsonlyjames commented 3 months ago

Oh for sure, I should have clarified that this is only in the case of needing to manually fix the issue. I don't suspect this to be a suitable solution for the plugin, but that maybe we can add to the documentation as a "how-to" of sorts for the interim until the core canvas issue is fixed. My apologies for insuiting it as a magic-fix-all!

Developer-Mike commented 3 months ago

Oh, and @TomBirkland feel free to reach out to me if you need a file fixed. I understand if you don't feel comfortable editing your file manually.

Developer-Mike commented 3 months ago

Just released version 3.0.4. It adds the auto-fix for broken canvas files.

itsonlyjames commented 3 months ago

Unreal work @Developer-Mike, you're the best 👏

TomBirkland commented 3 months ago

This is great, MIke. Everything appears to be working fine. Thanks!

TomBirkland commented 3 months ago

Uh, oh, sorry to jump back in here, but I noticed that if I now try to create a new canvas while this plug-in is enabled I get the

Failed to open ""

error. The workaround appears to be to disable the plug in, create the file, and then reenable the plugin. Another workaround is to make a copy of an existing Canvas and rename it.

I noticed that with the plug-in enabled, Obsidian will create a .canvas file (such as untitled1.canvass" but opening the created file with a text editor reveals a blank file. Another workaround I found was to

I was able to reproduce the error on both Mac and PC

Sorry! I know you've been working hard to squash this bug! I really appreciate your help. If I am doing something wrong please let me know!

Developer-Mike commented 2 months ago

Hey @TomBirkland. First of all, thank you soo much for your generous donation 🙏🏻

This issue already got reported (#38) and will be fixed today 👍🏻

Developer-Mike commented 2 months ago

@TomBirkland The issue #38 got fixed in version 3.0.5

Developer-Mike commented 2 months ago

This issue will be resolved in Obsidian version 1.7 (no ETA yet) according to the Obsidian team (https://forum.obsidian.md/t/canvas-file-corruption-if-file-node-has-custom-property-of-type-object-or-list/84698/6)

TomBirkland commented 2 months ago

Great! Thanks for all the updates.