Bug prevents rendering in Safari

[rodmachen]

A recent bump of a nested dependency caused our Backstage instance to fail in Safari. This could happen to a future deployment of the opensource Backstage app.

Expected Behavior

Safari should be supported by Backstage but this bug prevents it from being used.

Current Behavior

It appears that if normalize-url gets bumped to 6.0.1, an "irregular regex" error happens, preventing any of the UI from displaying in Safari. This version gets pulled in when parse-url gets bumped from 5.0.2 to 5.0.3. parse-url is used by git-up which is used by git-url-parse which is used by several OS components including @backstage/plugin-catalog and @backstage/plugin-scaffolder.

Possible Solution

We are using a yarn resolution to pin parse-url to 5.0.2 but that doesn't seem like a good long-term strategy. No other fix has been found.

Steps to Reproduce

1. yarn install installs normalize-url@6.0.1
2. Backstage appears as a blank page in Safari.

Context

Safari is only ~5% of our users, but it still needs to be supported.

Your Environment

• NodeJS Version (v12)
• Operating System and Version: Big Sur Mac OS 11.4
• Browser Information: Safari Version 14.1.1

[OrkoHunter]

Thanks for the detailed report @rodmachen !

Reproducible on the master branch of backstage/backstage as well by removing yarn.lock and running yarn install again. The reason why it's not reproducible as of now is because the current yarn.lock installs "normalize-url@3.3.0". But after removing it and re-installing afresh, the "normalize-url@6.x" gets pulled in.

Added a comment here https://github.com/IonicaBizau/parse-url/pull/20#issuecomment-861525603 saying that the upgrade was a breaking change.

@mtlewis has a suggestion that we can experiment with adding resolutions in our plugin libraries (catalog, scaffolder) to prevent the version bump and hope that fixes for all the Backstage instances. But not sure if that would work!

[rodmachen]

@OrkoHunter I wonder if this was fixed by the following PR? https://github.com/IonicaBizau/parse-url/pull/28

[OrkoHunter]

I think so too. parse-url@5.0.* is using the correct version of normalize-url v4 which is not broken on Safari.

https://github.com/IonicaBizau/parse-url/blob/5.0.6/package.json#L35

[egnwd]

Based off of the possible solution given, I used:

  "resolutions": {
    "**/git-up": "4.0.2"
  },

in my top level package.json and that has worked around the issue for me until something better comes along, doing it at the parse-url package level gave a warning so I went with git-up. In case it's useful for other passers-by.

[alexcurtin]

This is still not working in Safari with the latest backstage component versions updated, just a heads up

[freben]

Yeah I dug around a bit again on this, and git-url-parse has is in a bit of a pickle since they have upgraded a dependency past a point where the maker of that dependency says that browser support has been dropped. So as long as git-url-parse doesn't downgrade (or get rid of) that dependency again, or the maker of the dependency suddenly changes their mind, this problem will not go away.

I poked at them a bit recently to see if we can get this moving remotely somehow, and also pinged the actual transitive problematic package maintainer about whether he'd be amenable to fixing the problem at its root.

[freben]

Update - i got a fix merged and released to remove the safari-breaking regex. Only to discover that they exclusively dist as ESM now which our dependency isn't set up to consume, being purely CJS and not having a build/transpile setup.

So uh, that's where we are right now. Not sure how to best proceed.

[mapleeit]

Because of @freben 's excellent work, I think right now the easiest solution could be add this to your package.json:

"resolutions": {
  "**/normalize-url": "^7.0.1"
}

Then reinstall the dependencies by yarn or npm install

[freben]

@mapleeit EDIT: This may actually be a functioning fix now that ESM support is in place.

[Erog38]

Bumping this since it's affecting us as well and I can't seem to get the above solution in place for the reasons @freben mentioned. Is there a possible fix here and/or cli/tool upgrades that I missed in the referenced issue/PR?

[DanielTibbing]

We were facing what we thought was the same issue, Safari was completely blank. In our case it the page loaded fine if we had the developer console open in Safari, a bit weird and also made it trickier to debug.

Based off the error we got in the console we, after a lot of debugging, deduced that it had to do with the "subjects.ts" file

What solved the issue for us was to change the way "observable" in the subjects.ts file was instantiated.

From:

export class BehaviorSubject<T>
  implements Observable<T>, ZenObservable.SubscriptionObserver<T>
{

...

  constructor(value: T) {
    this.currentValue = value;
  }

  private readonly observable = new ObservableImpl<T>(subscriber => {
    if (this.isClosed) {
      if (this.terminatingError) {
        subscriber.error(this.terminatingError);
      } else {
        subscriber.complete();
      }
      return () => {};
    }

    subscriber.next(this.currentValue);

    this.subscribers.add(subscriber);
    return () => {
      this.subscribers.delete(subscriber);
    };
  });

...

}  

To:

export class BehaviorSubject<T>
  implements Observable<T>, ZenObservable.SubscriptionObserver<T>
{

...

  private readonly observable;

  constructor(value: T) {
    this.currentValue = value;
    this.observable = new ObservableImpl<T>(subscriber => {
      if (this.isClosed) {
        if (this.terminatingError) {
          subscriber.error(this.terminatingError);
        } else {
          subscriber.complete();
        }
        return () => {};
      }

      subscriber.next(this.currentValue);

      this.subscribers.add(subscriber);
      return () => {
        this.subscribers.delete(subscriber);
      };
    });
  }

...

}

In other words pretty much just moving the instantiation of observable into the constructor.

Pretty sure this is unrelated to the error described in this ticket, but since the symptoms are the same I figured I'd post about how we solved our issue here. People who are experiencing the same issue we had might find their way here!

As far as what the cause of the error is, I'm not sure. I'm guessing safari looks at minified code slightly differently compared to chrome and firefox? It is however very confusing that it works without error if you have the developer tools open 🤷

[freben]

Oh wow, that's obscure! Thanks for digging into it. Would you like to contribute that change in a pull request? Should be safe since they are functionally equivalent.

[DanielTibbing]

Oh wow, that's obscure! Thanks for digging into it. Would you like to contribute that change in a pull request? Should be safe since they are functionally equivalent.

Sure :) Been meaning to contribute for a while now, this is a good time as any to get started

[Crevil]

I wanted to update this issue as this is the goto based on the other closed issues. We see the same issue as reported in https://github.com/backstage/backstage/issues/8515#issuecomment-997915113 when pinning normalize-url to 7.0.1.

The frontend can load, but the backend cannot start.

/Users/bjornhaldsorensen/lunar/lunar-backstage/packages/backend/dist/main.js:598
/******/            throw e;
                    ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/bjornhaldsorensen/lunar/lunar-backstage/node_modules/normalize-url/index.js
require() of ES modules is not supported.
require() of /Users/bjornhaldsorensen/lunar/lunar-backstage/node_modules/normalize-url/index.js from /Users/bjornhaldsorensen/lunar/lunar-backstage/node_modules/parse-url/lib/index.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename /Users/bjornhaldsorensen/lunar/lunar-backstage/node_modules/normalize-url/index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /Users/bjornhaldsorensen/lunar/lunar-backstage/node_modules/normalize-url/package.json.

    at new NodeError (internal/errors.js:322:7)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1102:13)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:93:18)
    at Object.<anonymous> (/Users/bjornhaldsorensen/lunar/lunar-backstage/node_modules/parse-url/lib/index.js:6:20)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32) {
  code: 'ERR_REQUIRE_ESM'
}

I have worked around it by pinning parse-url to 5.0.0 instead.

[agates4]

My fix is to add

  "resolutions": {
    "**/parse-url": "5.0.0",
    "**/git-up": "4.0.2"
  }

To the top level package.json

[jselleck-sans]

We ran into this issue with the most recent version:bump. @agates4 top level package.json (above) fixed for now.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[webark]

Asking partially to try to prevent this from closing, but would be nice if this one stayed open at least so people can discover the workaround with the pinned resolution.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[quentinms]

This is still an issue, please leave this open Github-bot :)

[benjdlambert]

Maybe there's a solution with patch-package or something here for safari users? It's not great I know, but this is just unfortunate position that we're in with these libraries. https://www.npmjs.com/package/patch-package

[mmonroe-vmware]

This definitely is still an existing issue and should not be closed.

[freben]

@mmonroe-vmware Would you have the ability to check if latest master still exhibits this problem on safari, after this change? https://github.com/backstage/backstage/pull/12397

[MrLotU]

@freben I'm just getting started with backstage and ran in to the Safari issue too. I just checked the latest master (f537c232b2469d846466daa666feb7065de207f3) and still get the error. Let me know if there's any helpful information I can provide.

[gifteconomist]

The resolutions put forward by agates4 work for us - but parse-url@v^5.0.0 contains a critical security vulnerability.

When bumping the package version to either the latest (7.0.2) or to ^6.0.1, where the vulnerability was patched, the Safari bug reoccurs.

Posting for visibility - the package resolution that fixed this issue for us is not secure.

[benjdlambert]

@gifteconomist could you raise this exact concern with the upstream library to track what you're seeing there? I wonder if they've just missed this?

[gifteconomist]

@benjdlambert The vulnerability has been fixed in later major versions of parse-url, but only the ^5.0.0 version, which contains the vulnerability, fixes the Safari issue.

[benjdlambert]

Yes, and that concern is what I mean you should bring up with the upstream library, that this is still broken in the later versions. It's not something that we control, so we should track this issue upstream to get if fixed, and then we can push out the version bump here.

[gifteconomist]

@benjdlambert Gotcha. Looks like they're working on it - https://github.com/IonicaBizau/parse-url/issues/32

[benjdlambert]

Interesting that if they're not going to backport the fix to packages without esm support we might need to wait for this: #12218 also

[acierto]

My fix is to add

  "resolutions": {
    "**/parse-url": "5.0.0",
    "**/git-up": "4.0.2"
  }

To the top level package.json

Great workaround, the only problem with that, that parse-url has critical vulnerability on this version :(

https://nvd.nist.gov/vuln/detail/CVE-2022-2216

[Aj-vrod]

FYI: the issue https://github.com/IonicaBizau/parse-url/issues/32 was closed and a new parse-url version was released (8.0.0) with a bump "normalize-url": "^7.0.3" and a fix for the esm errors. The regex fix is on the way https://github.com/privatenumber/parse-url/pull/1

[jhaals]

Sounds like this ready to go! Does anyone want to pick this up? 🙏

[benjdlambert]

@jhaals upon further inspection it looks like we're still waiting for https://github.com/privatenumber/parse-url/pull/1 to make it to upstream. And then it should be ready I believe?

[Pike]

I think you pointed to a fork? I dug in a little bit and came across https://github.com/IonicaBizau/git-up/pull/34

[benjdlambert]

@Pike, yeah his fork is awaiting feedback from the original author and then he was gonna raise a PR to the upstream library. Not sure if that's a dependency on this ticket though or if your linked PR is enough :pray:

[Pike]

The regex is in a PR upstream, https://github.com/IonicaBizau/parse-url/pull/59.

[AjkayAlan]

It looks like the fix in the upstream was merged, and added to parse-url@8.1.0 which was cut this morning per https://github.com/IonicaBizau/parse-url/pull/65.

Could this by chance make it to backstage@v1.6.0?

[benjdlambert]

I've raised the above PR to bump all plugins in this repo, but any other plugins will also need bumping if they depend on it directly, or depend on some of our packages with an outdated version too :pray:

[per-oestergaard]

Hi. Would it be possible to detect Safari and return a simple text document with 'Please switch to xxx'? This would avoid wasting time when this problem is re-discovered.

[Crevil]

Isn't it fixed in the latest release?

[freben]

It is meant to be, yeah. If it isn't, we're of course happy to have it reported separately with more additional details.

[alexef]

run into this today, latest backstage, safari 16.1. hard to follow the thread, what is the latest resolution? SyntaxError: Invalid regular expression: invalid group specifier name

[benjdlambert]

@alexex I think that the fix is to bump git-url-parse if I remember correctly. If you do yarn why git-url-parse you should be able to see some duplicates and it's possible that some of those are breaking. If this isn't the case, then it's possible that this is another bug that we should open another ticket for as something else it causing that same error to be thrown but it's a different cause.

[alexef]

thanks, I have:

yarn why git-url-parse
yarn why v1.22.19
[1/4] 🤔  Why do we have the module "git-url-parse"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "git-url-parse@13.1.0"
info Reasons this module exists
   - "_project_#lerna" depends on it
   - Hoisted from "_project_#lerna#git-url-parse"
   - Hoisted from "_project_#@backstage#backend-common#git-url-parse"
   - Hoisted from "_project_#@backstage#cli#git-url-parse"
   - Hoisted from "_project_#lerna#@lerna#legacy-package-management#git-url-parse"
   - Hoisted from "_project_#@backstage#backend-common#@backstage#integration#git-url-parse"
   - Hoisted from "_project_#backend#@backstage#plugin-catalog-backend-module-github#git-url-parse"
   - Hoisted from "_project_#backend#@backstage#plugin-catalog-backend#git-url-parse"
   - Hoisted from "_project_#app#@backstage#plugin-catalog-import#git-url-parse"
   - Hoisted from "_project_#app#@backstage#plugin-github-actions#git-url-parse"
   - Hoisted from "_project_#backend#@backstage#plugin-scaffolder-backend#git-url-parse"
   - Hoisted from "_project_#app#@backstage#plugin-scaffolder#git-url-parse"
   - Hoisted from "_project_#app#@backstage#plugin-techdocs-module-addons-contrib#git-url-parse"
   - Hoisted from "_project_#app#@backstage#plugin-techdocs#git-url-parse"
   - Hoisted from "_project_#backend#@roadiehq#scaffolder-backend-module-http-request#@backstage#backend-common#git-url-parse"
   - Hoisted from "_project_#backend#@k-phoen#backstage-plugin-announcements-backend#@backstage#backend-common#git-url-parse"
   - Hoisted from "_project_#backend#@backstage#plugin-search-backend-module-techdocs#@backstage#plugin-techdocs-node#git-url-parse"
info Disk size without dependencies: "44KB"
info Disk size with unique dependencies: "68KB"
info Disk size with transitive dependencies: "228KB"
info Number of shared dependencies: 1
✨  Done in 0.61s.

and

grep git-url-parse yarn.lock 
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "^13.0.0"
    git-url-parse "13.1.0"
git-url-parse@13.1.0, git-url-parse@^13.0.0:
  resolved "https://registry.yarnpkg.com/git-url-parse/-/git-url-parse-13.1.0.tgz#07e136b5baa08d59fabdf0e33170de425adf07b4"
    git-url-parse "13.1.0"

so I'm guessing this is a new issue, I'll create one

[benjdlambert]

Can you do the same for parse-url too?

[ljupchokotev]

We seem to have started hitting the issue as well.

❯ yarn why parse-url                                                                                                                            
└─ git-up@npm:7.0.0
   └─ parse-url@npm:8.1.0 (via npm:^8.1.0)

❯ yarn why git-url-parse                                                                                                                     
├─ @backstage/backend-common@npm:0.19.0
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/backend-common@npm:0.19.0 [36a01]
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/backend-common@npm:0.19.0 [aa597]
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/cli@npm:0.22.8
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/cli@npm:0.22.8 [36a01]
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/integration@npm:1.5.0
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-catalog-backend-module-github@npm:0.3.1
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-catalog-backend@npm:1.10.0
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-catalog-import@npm:0.9.9
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-catalog-import@npm:0.9.9 [f87a9]
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-github-actions@npm:0.6.0
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-github-actions@npm:0.6.0 [f87a9]
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-techdocs-module-addons-contrib@npm:1.0.14
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-techdocs-module-addons-contrib@npm:1.0.14 [f87a9]
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-techdocs-node@npm:1.7.2
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-techdocs@npm:1.6.4
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
├─ @backstage/plugin-techdocs@npm:1.6.4 [f87a9]
│  └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)
│
└─ plugin-team-insights-backend@workspace:plugins/team-insights-backend
   └─ git-url-parse@npm:13.1.0 (via npm:^13.0.0)

[benjdlambert]

@ljupchokotev can you check https://github.com/backstage/backstage/issues/19266 instead