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

[master branch only] UI crashes when clicking node with an error message: Buffer is not defined #11764

Closed
2 of 3 tasks
toyamagu-2021 opened this issue Sep 6, 2023 · 10 comments · Fixed by #11766
Closed
2 of 3 tasks
Assignees
Labels
area/ui type/bug type/regression Regression from previous behavior (a specific type of bug)

Comments

@toyamagu-2021
Copy link
Member

toyamagu-2021 commented Sep 6, 2023

Pre-requisites

  • I have double-checked my configuration
  • I can confirm the issues exists when I tested with :latest
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what you expected to happen?

THIS DOES NOT RELATES TO RELEASED IMAGES either v3.5.0-rc nor v3.10.0.
Might be local development only?

  • Summary
    • UI crashes when we click a node hello1.
    • Curiously, step-mnss7 is ok.
    • image
    • image
  • Procedure
    • checkout master branch
    • make start UI=true
  • My Env
    • WSL
    • k3d

Version

master

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: steps-
spec:
  entrypoint: hello-hello-hello

  # This spec contains two templates: hello-hello-hello and whalesay
  templates:
  - name: hello-hello-hello
    # Instead of just running a container
    # This template has a sequence of steps
    steps:
    - - name: hello1            # hello1 is run before the following steps
        template: whalesay
        arguments:
          parameters:
          - name: message
            value: "hello1"
    - - name: hello2a           # double dash => run after previous step
        template: whalesay
        arguments:
          parameters:
          - name: message
            value: "hello2a"
      - name: hello2b           # single dash => run in parallel with previous step
        template: whalesay
        arguments:
          parameters:
          - name: message
            value: "hello2b"

  # This is the same template as from the previous example
  - name: whalesay
    inputs:
      parameters:
      - name: message
    container:
      image: docker/whalesay
      command: [cowsay]
      args: ["{{inputs.parameters.message}}"]


### Logs from the workflow controller

```text
kubectl logs -n argo deploy/workflow-controller | grep ${workflow}

Logs from in your workflow's wait container

kubectl logs -n argo -c wait -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded
@terrytangyuan
Copy link
Member

Might be related to recent dependency updates? cc @agilgur5

@toyamagu-2021
Copy link
Member Author

I'm not completely sure, but aac4f89 might cause this problem.
Previous commits are ok.

@agilgur5
Copy link
Member

agilgur5 commented Sep 6, 2023

I can check if I can reproduce. All of my PRs I had no errors locally, but maybe when mixed with dependabot updates something broke.

@agilgur5
Copy link
Member

agilgur5 commented Sep 6, 2023

I'm not completely sure, but aac4f89 might cause this problem.
Previous commits are ok.

Buffer is not defined

So #11628 upgraded to Webpack v5, which no longer auto-adds Node polyfills (this is normally fine for browsers b/c browser code shouldn't rely on Node). Specifically, I believe Buffer is one of them.
In #11418 (comment) I got a build error on Buffer not resolving. That was because the dependency was not resolving to the browser variant (due to outdated tsconfig, typescript, etc)

This is a run-time error though, so that suggests that a dep might be using Buffer as a global (i.e. did not explicitly import it) 🤔

@agilgur5
Copy link
Member

agilgur5 commented Sep 6, 2023

Oh. That is internal code using Buffer:

const data = Buffer.from(input);
.

That probably requires a polyfill now. I'll look into that.

@agilgur5 agilgur5 added the type/regression Regression from previous behavior (a specific type of bug) label Sep 6, 2023
@agilgur5
Copy link
Member

agilgur5 commented Sep 6, 2023

TIL what an FNV hash is. This comes from 2 year old code in #6925, which was quite literally intended to duplicate server-side code per #6865 (comment).

So that makes a bit more sense why server-side JS is being used 😅

Ideally we might be able to replace the hash function with a library that works client-side, but if not, will add a polyfill

@agilgur5 agilgur5 self-assigned this Sep 6, 2023
@toyamagu-2021
Copy link
Member Author

toyamagu-2021 commented Sep 6, 2023

Great! Thank you!

Just in case, this regression is not included in v3.4.11.
(It's obvious considering problematic commit is not included in cherry-pick candidate.)
I've checked this by checking out to release-3.4.11 and create local environment.

@terrytangyuan
Copy link
Member

Yeah I intentionally didn't include those UI changes since they are relatively new and not thoroughly tested yet. Thanks for confirming.

@agilgur5
Copy link
Member

agilgur5 commented Sep 6, 2023

Yea the UI build system being upgraded a major version is definitely more of a risky change and certainly not a bugfix 😅

Idk how long 3.4.x will get backports for (this is the first minor release I've been part of), but if it's a while, we may eventually want to add some of the UI deps changes so that it doesn't diverge too far (as that could heavily complicate UI backports).

@agilgur5
Copy link
Member

agilgur5 commented Sep 6, 2023

Managed to find a pretty tiny fix that didn't need a polyfill or another library.

See #11766

@agilgur5 agilgur5 changed the title [master branch only] UI crashes when clicking node with an error message: Buffer is not defined [master branch only] UI crashes when clicking node with an error message: Buffer is not defined Jul 12, 2024
@argoproj argoproj locked as resolved and limited conversation to collaborators Jul 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/ui type/bug type/regression Regression from previous behavior (a specific type of bug)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants