Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle loading and parsing errors when loading themes #5669

Merged
merged 2 commits into from
Aug 13, 2021

Conversation

rpocklin
Copy link
Contributor

@rpocklin rpocklin commented Aug 13, 2021

Description

There are multiple conditions where a custom theme will fail to load eg.

  1. If the file extension does not end in .json
  2. If the file is not found
  3. If there is any other error attempting to fetch the file from network
  4. If the file is not a valid JSON file format

Currently only (1), (2) and (3) are handled by returning the default owncloud extension.

This fix adds error handling for (4) and also refactors the code a little. Currently (4) will break the application with:

web-runtime.js:5504 SyntaxError: Unexpected token d in JSON at position 584

as there is no catch around the json parsing.

With the fix, it will show the following on the console:

web-runtime.js:5262 Failed to load theme 'themes/aarnet/theme.json' is not a valid JSON file, using default theme.

This fix also logs a console error that a custom theme load failed and the default theme is going to be used (I personally found it confusing as it failed silently to load the theme and I was unsure what had happened.)

How Has This Been Tested?

Tested manually in the browser and have added / updated unit tests.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

@update-docs
Copy link

update-docs bot commented Aug 13, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@ownclouders
Copy link
Contributor

Results for oC10SharingInternalGroupsSharingIndicator https://drone.owncloud.com/owncloud/web/18261/24/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Nice improvement for admins, love it! 💪🏻

@kulmann
Copy link
Member

kulmann commented Aug 13, 2021

I restarted CI, since the failing test was a random failure... we have a lot of those recently and currently work towards fixing it, sorry.

@sonarcloud
Copy link

sonarcloud bot commented Aug 13, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

32.7% 32.7% Coverage
4.0% 4.0% Duplication

@ownclouders
Copy link
Contributor

Results for oC10SharingInternalUsersSharingIndicator https://drone.owncloud.com/owncloud/web/18267/28/1
The following scenarios passed on retry:

  • webUISharingInternalUsersToRootSharingIndicator/shareWithUsers.feature:98

@ownclouders
Copy link
Contributor

Results for oC10XGAPortrait1 https://drone.owncloud.com/owncloud/web/18267/42/1
The following scenarios passed on retry:

  • webUIDeleteFilesFolders/deleteFilesFolders.feature:59

@kulmann kulmann merged commit ce2d195 into owncloud:master Aug 13, 2021
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