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

[BUG] Startup fails with theme overrides #6704

Open
BionIT opened this issue May 2, 2024 · 11 comments · Fixed by #6703 · May be fixed by #6730
Open

[BUG] Startup fails with theme overrides #6704

BionIT opened this issue May 2, 2024 · 11 comments · Fixed by #6703 · May be fixed by #6730
Assignees
Labels
bug Something isn't working v2.16.0

Comments

@BionIT
Copy link
Collaborator

BionIT commented May 2, 2024

Describe the bug

A clear and concise description of what the bug is.
Screenshot 2024-05-02 at 12 15 07 PM
Screenshot 2024-05-02 at 12 15 15 PM
Screenshot 2024-05-02 at 12 15 25 PM

To Reproduce
Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior
A clear and concise description of what you expected to happen.

OpenSearch Version
Please list the version of OpenSearch being used.

Dashboards Version
Please list the version of OpenSearch Dashboards being used.

Plugins

Please list all plugins currently enabled.

Screenshots

If applicable, add screenshots to help explain your problem.

Host/Environment (please complete the following information):

  • OS: [e.g. iOS]
  • Browser and version [e.g. 22]

Additional context

Add any other context about the problem here.

@BionIT BionIT added bug Something isn't working untriaged labels May 2, 2024
@BionIT
Copy link
Collaborator Author

BionIT commented May 2, 2024

checkout a commit older than this PR would resolve thus narrow down to this PR #5652

@kavilla kavilla removed the untriaged label May 2, 2024
@kavilla kavilla added the v2.14.0 label May 2, 2024
@kavilla kavilla linked a pull request May 2, 2024 that will close this issue
7 tasks
@kavilla kavilla changed the title [BUG] Broken feature [BUG] Themes user specific invalid key May 2, 2024
@BionIT
Copy link
Collaborator Author

BionIT commented May 2, 2024

Different from the screen recording in the PR #6703

The dashboard in future playground won't display anything

@BionIT BionIT reopened this May 3, 2024
@BionIT
Copy link
Collaborator Author

BionIT commented May 3, 2024

Found issue still happening, thus re-opened

@joshuarrrr
Copy link
Member

@BionIT Can you provide some repro details? If I visit https://future.playground.opensearch.org, this is what I see
Screenshot 2024-05-02 at 5 40 48 PM

@BionIT
Copy link
Collaborator Author

BionIT commented May 3, 2024

Yep, @joshuarrrr this is after switching to an older commit than the PR which introduced the bug, otherwise, we see empty page. Please check future playground for the empty page and error now.

@joshuarrrr
Copy link
Member

I just need more info to be able to debug it, so if there was a version that included #6703 and #5652, that would be really helpful, because it doesn't repro running locally. I can also provide some additional context:

The issue is likely not in /bootstrap.js (source: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/legacy/ui/ui_render/bootstrap/bootstrap.js.hbs ), despite that being the failure point. To start up correctly, that application requires /startup.js ( source: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/legacy/ui/ui_render/bootstrap/startup.js.hbs )to successfully load and execute first. You can see in the console that startup.js failed on an unexpected token on L15: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/legacy/ui/ui_render/bootstrap/startup.js.hbs#L15

That means that there's likely an issue with this block of code:

const configDarkMode =
!authEnabled || request.auth.isAuthenticated
? await uiSettings.get('theme:darkMode')
: uiSettings.getOverrideOrDefault('theme:darkMode');

which wasn't edited, but moved in #5652

@ananzh
Copy link
Member

ananzh commented May 3, 2024

@BionIT same question here, is there a way to reproduce locally or could you help to add some repro steps?

@BionIT
Copy link
Collaborator Author

BionIT commented May 3, 2024

To repro:

  • add the commit, see empty page and error in future playground
  • remove the commit, OSD bootstraps correctly in playground

I don't have bandwidth to look further, and I wonder if the change would behave different when it is in container?

@kavilla
Copy link
Member

kavilla commented May 3, 2024

i think cache issue. that supercedes the cache buster for plugins.

2.13 navigate to playground/app/plugin
HTML dom gets loaded with only reference to script bootstrap
HTML dom downloads new bootstrap script due to cache busting mechanism
bootstrap statically sets that window osdThemeTag

2.14 navigate to playground/app/plugin
HTML dom gets loaded from cache since visiting again. but it only has the reference to bootstrap, and does not include startup.js
HTML dom downloads new bootstrap script due to cache busting mechanism
bootstrap expected startup.js to be downloaded and ran to set that global window value. get undefined exception

expected:
HTML dom gets loaded with both startup and bootstrap.

tbh i'm not sure if this normal issue or something special about docker's caching optimization. we could consider just adding a default value to that osdThemeTag in bootstrap if window.osdThemeTag is undefined. @joshuarrrr @ananzh.

ananzh added a commit to ananzh/OpenSearch-Dashboards that referenced this issue May 3, 2024
Issue Resolve
opensearch-project#6704

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
@joshuarrrr joshuarrrr changed the title [BUG] Themes user specific invalid key [BUG] Startup fails with theme overrides May 7, 2024
@joshuarrrr joshuarrrr linked a pull request May 7, 2024 that will close this issue
7 tasks
@kavilla
Copy link
Member

kavilla commented May 7, 2024

reverted in 2.14 in 2.x as @joshuarrrr is addressing existing issue that got exposed by this work but not from this work.

@kavilla kavilla added v2.15.0 and removed v2.14.0 labels May 7, 2024
@BionIT
Copy link
Collaborator Author

BionIT commented Jun 4, 2024

@joshuarrrr Do we still target this fix for 2.15 or 2.16?

@ananzh ananzh added v2.16.0 and removed v2.15.0 labels Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v2.16.0
Projects
None yet
4 participants