Merge pull request #5669 from rpocklin/fix-handle-theme-load-failures

changelog/unreleased/bugfix-handle-errors-when-loading-themes

@@ -0,0 +1,5 @@
+Bugfix: Handle loading and parsing errors when loading themes
+
+Adds graceful error handling of json parse errors when loading custom themes.
+
+https://github.com/owncloud/web/pull/5669

packages/web-runtime/src/helpers/theme.js

@@ -4,20 +4,21 @@ export const loadTheme = async (location = '') => {
   const defaults = { theme: defaultTheme }
 
   if (location.split('.').pop() !== 'json') {
+    console.error(`Theme '${location}' does not specify a json file, using default theme.`)
     return defaults
   }
 
-  let response
   try {
-    response = await fetch(location)
+    const response = await fetch(location)
+    if (!response.ok) {
+      return defaults
+    }
+    const theme = await response.json()
+    return { theme }
   } catch (e) {
+    console.error(
+      `Failed to load theme '${location}' is not a valid json file, using default theme.`
+    )
     return defaults
   }
-
-  if (!response.ok) {
-    return defaults
-  }
-
-  const theme = await response.json()
-  return { theme }
 }

packages/web-runtime/tests/unit/helpers/theme.spec.js

@@ -4,25 +4,35 @@ import merge from 'lodash-es/merge'
 
 describe('theme loading and error reporting', () => {
   beforeEach(() => {
-    fetch.resetMocks()
+    global.console = { error: jest.fn() }
+  })
+  afterEach(() => {
+    jest.clearAllMocks()
   })
 
   it('should load the default theme if location is empty', async () => {
     const { theme } = await loadTheme()
     expect(theme).toMatchObject(defaultTheme)
   })
 
-  it('should load the default theme if location is not a json file', async () => {
+  it('should load the default theme if location is not a json file extension', async () => {
     const { theme } = await loadTheme('some_location_without_json_file_ending.xml')
     expect(theme).toMatchObject(defaultTheme)
   })
 
-  it('should load the default theme if location errors', async () => {
+  it('should load the default theme if location is not found', async () => {
     fetch.mockResponse(new Error(), { status: 404 })
     const { theme } = await loadTheme('http://www.owncloud.com/unknown.json')
     expect(theme).toMatchObject(defaultTheme)
   })
 
+  it('should load the default theme if location is not a valid json file', async () => {
+    const customTheme = merge({}, defaultTheme, { default: { logo: { login: 'custom.svg' } } })
+    fetch.mockResponse(JSON.stringify(customTheme) + '-invalid')
+    const { theme } = await loadTheme('http://www.owncloud.com/invalid.json')
+    expect(theme).toMatchObject(defaultTheme)
+  })
+
   it('should load the default theme if server errors', async () => {
     fetch.mockReject(new Error())
     const { theme } = await loadTheme('http://www.owncloud.com')