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

[QA] blank page with trivial syntax errors in config.json #4636

Closed
jnweiger opened this issue Jan 20, 2021 · 6 comments · Fixed by #4749
Closed

[QA] blank page with trivial syntax errors in config.json #4636

jnweiger opened this issue Jan 20, 2021 · 6 comments · Fixed by #4749
Assignees
Labels

Comments

@jnweiger
Copy link
Contributor

Seen with web-1.0.2

Follow the instructions at https://github.com/owncloud/QA/blob/master/Server/Test_Plan_web.md

but introduce a trivial syntax error in config.json -- e.g. a trailing comma , before closing } brace:

      "title": { "en": "Classic Design", "de": "Dateien", },
  • the oauth2 authorization is performed,
  • then the browser is redirected to a blank web page.
  • server has no error messages, neither in the docker logs output, nor in owncloud.log with log verbosity "Everything".
  • firefox console reports (only)
    image

Expected behaviour: The json syntax error is reported. Somewhere.

@micbar
Copy link
Contributor

micbar commented Jan 21, 2021

Not so easy. The same happens when you add a comment in the json file.

@kulmann @LukasHirt We could add config.json validation, but please be sure that this doesn't happen on every request.

@LukasHirt
Copy link
Collaborator

but please be sure that this doesn't happen on every request.

We fetch the config once when Web is opened:

web/src/web.js

Line 214 in 5878eeb

config = await fetch('config.json')

So it should be already safe to do the validation and not be afraid of having it in every request. The config is then afterwards saved to store.

@LukasHirt
Copy link
Collaborator

Something to consider https://www.npmjs.com/package/jsonschema

@individual-it
Copy link
Member

individual-it commented Feb 1, 2021

ToDo QA-Team:

  • add unit tests for this case

@LukasHirt
Copy link
Collaborator

add UI tests for this case

@individual-it Would it be possible to get this done with unit tests?

@individual-it
Copy link
Member

good thought, I haven't looked into what they currently doing but I've changed the todo above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants