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

@sentry/ember throws exception when handling rootURLs #2977

Closed
5 of 9 tasks
jonchay opened this issue Oct 15, 2020 · 11 comments
Closed
5 of 9 tasks

@sentry/ember throws exception when handling rootURLs #2977

jonchay opened this issue Oct 15, 2020 · 11 comments
Assignees

Comments

@jonchay
Copy link

jonchay commented Oct 15, 2020

Package + Version

  • @sentry/browser
  • @sentry/node
  • raven-js
  • raven-node (raven for node)
  • other: @sentry/ember

Version:

5.26.0

Description

I'm using @sentry/ember package, in our Ember application we specify a rootURL for our ember application to host on that specific path (e.g. https://www.domain.com/ember-app/)

When we ran the application an error throws from sentry-performance from this line:

Error: Assertion Failed: You must pass a url that begins with the application's rootURL "/ember-app/"

But the url it was handling is /. If I'm not mistaken, when we pass a url to routerService.recognize we have to prefix with the rootURL.

@netes
Copy link

netes commented Nov 10, 2020

I stumbled upon the same problem today. Is there any ETA for fixing this?

@patrickberkeley
Copy link

Came across this issue today as well.

@theklr
Copy link

theklr commented Nov 24, 2020

Ditto as an issue

@jonchay
Copy link
Author

jonchay commented Jan 7, 2021

Hello, just want to follow-up if there is any resolution to this issue?

k-fish added a commit that referenced this issue Jan 11, 2021
Mentioned in #2977, route recognition needs the url to be correctly formatted. Since there are different location types, using formatURL should get the correct URL out for use in recognition.
kamilogorek added a commit that referenced this issue Jan 13, 2021
* fix(ember): Fix rootURL breaking route recognition

Mentioned in #2977, route recognition needs the url to be correctly formatted. Since there are different location types, using formatURL should get the correct URL out for use in recognition.

* Update packages/ember/addon/instance-initializers/sentry-performance.ts

Co-authored-by: Kamil Ogórek <kamil.ogorek@gmail.com>
@Gorzas
Copy link

Gorzas commented Jan 21, 2021

I still have the problem with the last fix. Shouldn't the code concat the rootURL? Like:

  let url = location && location.getURL && location.formatURL && location.formatURL(location.getURL());
  url = `${location.rootURL}${url}`;

@k-fish
Copy link
Member

k-fish commented Jan 21, 2021

@Gorzas what kind of location do you have? (auto, hash etc.), I can test it out if you give me an example. When I checked the source, formatURL should be adding rootURL already (see: https://github.com/emberjs/ember.js/blob/b9c974c7f7fe086deb8a5ec0150f7c1877dfdb9b/packages/%40ember/-internals/routing/lib/location/history_location.ts#L251)

@Gorzas
Copy link

Gorzas commented Jan 21, 2021

@k-fish I'm using hash, so it uses hash_location instead of history_location. You can see the difference: https://github.com/emberjs/ember.js/blob/v3.24.0/packages/%40ember/-internals/routing/lib/location/hash_location.ts#L153-L155

@k-fish
Copy link
Member

k-fish commented Jan 22, 2021

@Gorzas ah yeah, hash locations don't append the root in their formatURL. I put up a special case for hash locations and tried it out on the addon's dummy application, it should work once #3195 goes in.

@Gorzas
Copy link

Gorzas commented Jan 25, 2021

Thanks @k-fish! Is there a way so I can test the solution? I don't know how to install from an specific commit in a monorepo.

@k-fish
Copy link
Member

k-fish commented Jan 25, 2021

@Gorzas if you'd like to test it out that'd be great! It's been a while since I've done it from scratch, but using the monorepo should be pretty much identical to testing a regular local package, with an extra step or two.

In the monorepo directory:

  • yarn run build (build the local and supporting packages)
  • yarn run link:yarn (makes the packages available for linking)

In your app directory:

  • ember install @sentry/ember (if you haven't already)
  • yarn link @sentry/ember

If you run into an error when starting the server about mismatching versions, just make sure your applications package.json has the latest version and it matches the monorepo's version (I just pushed the new version, so they should be both 6.0.2)

@Gorzas
Copy link

Gorzas commented Jan 26, 2021

@k-fish it's working now. Thanks a lot! 👍

@k-fish k-fish closed this as completed Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants