fix: improve error response when loading XML#3545
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves diagnostics and cleanup behavior when loading workspace XML fails, by wrapping Blockly.Xml.domToWorkspace errors with additional context and ensuring workspace resize + event grouping state is restored even on failure.
Changes:
- Wrap
domToWorkspacefailures with an error message that includes top-level block/shadow element identifiers. - Ensure
Blockly.Events.setGroup(false)andworkspace.setResizesEnabled(true)always run via afinallyblock.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cwillisf
force-pushed
the
fix/xml-load-diagnostics
branch
from
0fac758 to
4c10154
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cwillisf
force-pushed
the
fix/xml-load-diagnostics
branch
from
4c10154 to
a53ed3b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…omXml When domToWorkspace throws, wrap the error with context listing the top-level XML elements that were being loaded. This makes it much easier to diagnose which block or element caused the failure. Also move setGroup(false) and setResizesEnabled(true) into a finally block so they are always called, even when loading fails. Previously setGroup(true) was never closed, and setResizesEnabled(true) was skipped on error.
cwillisf
force-pushed
the
fix/xml-load-diagnostics
branch
from
a53ed3b to
904cd6b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Resolves
Improves, but does not fix, #3543
See also scratchfoundation/scratch-editor#528
Proposed Changes
When
domToWorkspacethrows, wrap the error with context listing the top-level XML elements that were being loaded. This makes it much easier to diagnose which block or element caused the failure.Also move
setGroup(false)andsetResizesEnabled(true)into a finally block so they are always called, even when loading fails. PreviouslysetGroup(true)was never closed, andsetResizesEnabled(true)was skipped on error.Test Coverage
NA - this is just about providing better feedback in error cases