Skip to content

Conversation

@mslynch
Copy link
Member

@mslynch mslynch commented Oct 8, 2025

Intent

Related to #3180.

When deploying in this situation, it results in an internal server error due to a nil pointer dereference. We're not checking the error we get from walking the filesystem soon enough.

Type of Change

    • Bug Fix
    • New Feature
    • Breaking Change
    • Documentation
    • Refactor
    • Tooling

Approach

Moved the error handling up so it can catch this case.

User Impact

Users won't hit an internal server error in this case; the filesystem walk will complete successfully and an error will be logged about the file that caused an error.

Automated Tests

Added a test to verify this case.

Directions for Reviewers

Use steps in linked ticket and hit deploy.

Checklist

  • I have updated the root CHANGELOG.md to cover notable changes.

@mslynch mslynch marked this pull request as ready for review October 8, 2025 18:10
Copy link
Collaborator

@marcosnav marcosnav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving up the error checking is a good move, although not sure this is the complete solution.

I tried this out and a dir with permissions 600 that is selected in the files tree is still included in the bundle as an empty dir.

There's a UX problem here too. If there are files in the configuration file for which the directory permissions change, it is not possible to de-select from the files tree since the dir won't expand. This is a very contrived issue and maybe not worth solving the UX side of it, specially if we manage to not include the directory on the deployment, which sounds like a good enough solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants