getsentry / sentry

Developer-first error tracking and performance monitoring
https://sentry.io
Other
38.6k stars 4.13k forks source link

Hyperlink for JS file is a 404 #71216

Open lforst opened 4 months ago

lforst commented 4 months ago

Originally posted by @alessandro-newzoo in https://github.com/getsentry/sentry-javascript/issues/11865#issuecomment-2115344655

TLDR: The problem seems to be gone on new issues concerning the same JS file. But, even though Sentry shows the correct line and content of the source file, the path it's showing is wrong and if I click on it, it gives me a 404.

Hi, I'm the person this issue was opened for šŸ‘‹šŸ»

@stayallive you're right, I'm working on our website's theme, and the content of the folder wp-content/themes/newzoo/ is the only part tracked by git and uploaded to GitHub, but the path mapping should be correct as other files within the same folder were mapping correctly.

Before this issue was opened there's been a long discussion between me and Stella via tickets, so you're all probably lacking a lot of context here.

Basically I had this issue in Sentry where wp-content/themes/newzoo/js/_dist/single--games.bundle failed to be mapped to its source file wp-content/themes/newzoo/js/_src/single--games.js: https://newzoo-com.sentry.io/share/issue/f56fc05f6b964da38d075cc4b110158f/

Other issues concerning files in the same folder as the above were mapping correctly, eg: https://newzoo-com.sentry.io/share/issue/00f6add9b4064aa685640939fa69ae87/

Although I'm only noticing now something odd. The two issues above show different paths to the source:

The issue that doesn't map, shows wp-content/themes/newzoo/js/_dist/single--games.bundle

The issue that maps correctly, shows wp-content/themes/newzoo/js/_dist/newzoo/js/_src/single--trendreports

ā†‘ this one, even though is the one that works, shows a wrong path to the source:

wp-content/themes/newzoo/js/_dist/newzoo/js/_src/single--trendreports
                            ^^^^^^^^^^^^^^^^
                         this shouldn't be here

To sum it up, the source files are at wp-content/themes/newzoo/js/_src the dist files at wp-content/themes/newzoo/js/_dist and Sentry thinks the source files are at wp-content/themes/newzoo/js/_dist/newzoo/js/_src/

So ironically the one that did not work was the one with the correct mapping. It's somehow appending part of the src path to the full dist path.

I suspect that's why that was happening, though I have no idea why since both files were generated by the same webpack config. Is this something onwp-sentry's end? Sentry's? Or something I need to fix?

Like I said above, I can see the correct content in the stack trace, but the path is not correct for some reason, so if I click on it I get a 404:

CleanShotX - Arc  16 05 2024 at 15-31-11 @2x CleanShotX - Arc  16 05 2024 at 15-42-39

I went ahead and deleted all Code Mappings, recreated it by clicking on Set up Code Mapping on the issue screen, which then created a new wrong mapping again (which works at showing the src content, but 404s on click):

https://github.com/getsentry/sentry-javascript/assets/47320294/39ddb4ca-1bfc-44b5-b26c-22e79cd63298

getsantry[bot] commented 4 months ago

Routing to @getsentry/product-owners-issues for triage ā²ļø

jangjodi commented 4 months ago

This is very odd behavior, @armenzg do you have any idea what might be happening here with the automatic code mappings?

armenzg commented 4 months ago

Hello @alessandro-newzoo for reporting. The hyperlink for the JS file is unrelated to the code mappings feature (the GitHub icon on the right side) [1].

If you load the JSON event, you will see abs_path as the link to the JS file:

image

Please click the GitHub icon and let me know if that URL works as expected.

[1] Hyperlink for JS files and GitHub links image

lforst commented 4 months ago

@alessandro-newzoo can you share the content of the sources field in your source map? I think that is generally where that path comes from.

alessandro-newzoo commented 3 months ago

Hi guys thank you all for your help with this!

Let's take this Sentry Issue as a reference for this discussion: https://newzoo-com.sentry.io/issues/5141223488


@armenzg yes, clicking on the GH icon works as expected and takes me to the correct line on my GH repo:

CleanShotX - Arc  23 05 2024 at 10-43-38 @2x


@lforst sure, here's the full wp-content/themes/newzoo/js/_dist/single--trendreports.bundle.js.map

"sources":["webpack://newzoo/./js/_src/single--trendreports.js"]
{
    "version": 3,
    "file": "single--trendreports.bundle.js",
    "mappings": "4gBAAAA,OAAOC,UAAUC,OAAOC,IACvB,MAAMC,EAAQD,EAAE,8CAEVE,EAAa,CAClBC,QAAS,oCACTC,MAAO,oBACPC,IAAK,gBACLC,QAAS,KACRC,KAAKC,GAAG,yCAA0C,CACjDC,SAAU,IACVC,EAAG,QACF,EAEHC,YAAa,KACZJ,KAAKC,GAAG,yCAA0C,CACjDC,SAAU,IACVC,EAAG,MACF,GAIEE,EAAYC,cAAcC,OAAOZ,GACvC,IAAIa,EAEJ,MAAMC,EAAgB,KACrB,MAAMC,EAAajB,EAAEC,GAAOiB,cAExBlB,EAAEmB,QAAQC,SAAWH,EAAa,GACrCF,EAAWM,SAEXN,EAAWO,SACZ,EAGDtB,EAAEmB,QAAQI,GAAG,UAAU,WACtBP,IACAD,EAAWS,UACXZ,EAAUY,SACX,IAGAL,OAAOM,gBAAkB,KACxB,MAAMC,EAAc,CACnBvB,QAASF,EACT0B,KAAK,EACLvB,MAAO,WACPwB,WAAY,uCACZvB,IAAK,WAAaL,EAAEC,GAAO4B,cAAgB,KAE5Cd,EAAaF,cAAcC,OAAOY,GAClCV,GAAe,CACf",
    "sources": [
        "webpack://newzoo/./js/_src/single--trendreports.js"
    ],
    "sourcesContent": [
        "jQuery(document).ready(($) => {\n\tconst $form = $('#single--trendreports--header--form--inner');\n\n\tconst ctaOptions = {\n\t\ttrigger: '#single--trendreports--body--form',\n\t\tstart: 'top bottom-=170px',\n\t\tend: 'bottom bottom',\n\t\tonEnter: () => {\n\t\t\tgsap.to('#single--trendreports--mobileStickyCta', {\n\t\t\t\tduration: 0.25,\n\t\t\t\ty: '100%',\n\t\t\t});\n\t\t},\n\t\tonLeaveBack: () => {\n\t\t\tgsap.to('#single--trendreports--mobileStickyCta', {\n\t\t\t\tduration: 0.25,\n\t\t\t\ty: '0%',\n\t\t\t});\n\t\t},\n\t};\n\n\tconst stickyCta = ScrollTrigger.create(ctaOptions);\n\tlet stickyForm;\n\n\tconst checkViewport = () => {\n\t\tconst formHeight = $($form).outerHeight();\n\n\t\tif ($(window).height() > formHeight + 50) {\n\t\t\tstickyForm.enable();\n\t\t} else {\n\t\t\tstickyForm.disable();\n\t\t}\n\t};\n\n\t$(window).on('resize', function () {\n\t\tcheckViewport();\n\t\tstickyForm.refresh();\n\t\tstickyCta.refresh();\n\t});\n\n\t// Fired from single--header.twig\n\twindow.headerFormReady = () => {\n\t\tconst formOptions = {\n\t\t\ttrigger: $form,\n\t\t\tpin: true,\n\t\t\tstart: 'top 20px',\n\t\t\tendTrigger: '#single--trendreports--body--article',\n\t\t\tend: 'bottom ' + ($($form).innerHeight() + 20),\n\t\t};\n\t\tstickyForm = ScrollTrigger.create(formOptions);\n\t\tcheckViewport();\n\t};\n});\n"
    ],
    "names": [
        "jQuery",
        "document",
        "ready",
        "$",
        "$form",
        "ctaOptions",
        "trigger",
        "start",
        "end",
        "onEnter",
        "gsap",
        "to",
        "duration",
        "y",
        "onLeaveBack",
        "stickyCta",
        "ScrollTrigger",
        "create",
        "stickyForm",
        "checkViewport",
        "formHeight",
        "outerHeight",
        "window",
        "height",
        "enable",
        "disable",
        "on",
        "refresh",
        "headerFormReady",
        "formOptions",
        "pin",
        "endTrigger",
        "innerHeight"
    ],
    "sourceRoot": ""
}
getsantry[bot] commented 3 months ago

Routing to @getsentry/product-owners-issues-source-maps for triage ā²ļø

lforst commented 3 months ago

@armenzg did you try out using raw_stacktrace.frames[].abs_path?

getsantry[bot] commented 3 months ago

Routing to @getsentry/product-owners-issues for triage ā²ļø

armenzg commented 3 months ago

@alessandro-newzoo we are wondering if this link causes more confusion than it helps:

image

since you have access to the source file via the GitHub icon:

image

The first hyperlink is mostly useful if there's no GitHub link.

alessandro-newzoo commented 3 months ago

Hi,

personally, I think they serve two different purposes and complement each other:

None of these points are crucial, but they're nice to have.

Thank you