Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

SvelteKit auto-instrumentation causes faulty JSX error #9318

Closed
3 tasks done
kenkunz opened this issue Oct 19, 2023 · 11 comments · Fixed by #15578
Closed
3 tasks done

SvelteKit auto-instrumentation causes faulty JSX error #9318

kenkunz opened this issue Oct 19, 2023 · 11 comments · Fixed by #15578
Assignees
Labels
Package: sveltekit Issues related to the Sentry SvelteKit SDK Type: Bug

Comments

@kenkunz
Copy link

kenkunz commented Oct 19, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io) as well as self-hosted (same issue occurs in both environments).

Which SDK are you using?

@sentry/sveltekit

SDK Version

7.74.1

Framework Version

1.26.0

Link to Sentry event

https://zenthropic.sentry.io/issues/4559398012/?project=4504934292258816

SDK Setup

Sentry.init({
  dsn: 'https://3365729f7ba44228b319f4ad7ae32e6a@o4504934289506304.ingest.sentry.io/4504934292258816',
  tracesSampleRate: 1.0,
});

Steps to Reproduce

I created a new minimum SvelteKit repro with Sentry to demonstrate this bug.

  1. Create new SvelteKit project
    npm create svelte@latest sentry-repro
    # - Skeleton project
    # - Yes, using TypeScript syntax
    # - skip other add-ons
    
    cd sentry-repro
    npm i
  2. Add Sentry using setup wizard
    npx @sentry/wizard@latest -i sveltekit
    # - Sentry SaaS
    # - yes
    # - select project
  3. Add a src/routes/+page.ts file with a TypeScript <> cast operator
    export async function load() {
      let x: unknown = 'foo';
      console.log(<string>x);
    }
  4. Start dev server and open the page
    npm run dev
    # http://localhost:5173/
  5. Note the error:
    Internal server error: Unterminated JSX contents. (3:22)
    
  6. Modify vite.config.ts to disable auto-instrumentation:
    // add sentrySvelteKit config option
    autoInstrument: false
  7. Note that the error goes away / page serves as expected

Expected Result

No error should occur at step 5.

Auto-instrumentation should not (incorrectly) interpret TypeScript cast operator <> as JSX.

Current work-around: using as cast operator instead of <>.

Actual Result

The following error occurs at step 5.

Unterminated JSX contents. (3:22)
SyntaxError: Unterminated JSX contents. (3:22)
    at constructor (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:356:19)
    at V8IntrinsicMixin.raise (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:3223:19)
    at V8IntrinsicMixin.jsxReadToken (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:6605:20)
    at V8IntrinsicMixin.getTokenFromCode (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:6947:12)
    at V8IntrinsicMixin.getTokenFromCode (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:9852:11)
    at V8IntrinsicMixin.nextToken (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:2347:10)
    at V8IntrinsicMixin.next (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:2258:10)
    at V8IntrinsicMixin.eat (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:2262:12)
    at V8IntrinsicMixin.expect (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:3590:10)
    at V8IntrinsicMixin.jsxParseOpeningElementAfterName (/Users/ken/Code/sentry-repro/node_modules/@babel/parser/lib/index.js:6836:10
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 19, 2023
@github-actions github-actions bot added the Package: sveltekit Issues related to the Sentry SvelteKit SDK label Oct 19, 2023
@lforst
Copy link
Member

lforst commented Oct 20, 2023

Hi, @kenkunz thanks for reporting this! Do you mind explaining what you mean by

TypeScript <> cast operator
console.log(<string>x);

it's the first time I've ever heard of this. The newest TS version doesn't compile here either: https://www.typescriptlang.org/play?#code/MYewdgziA2CmB00QHMAUAeCAXATgSzGQD4APASgG4AoIA

Are you sure this is valid syntax?

@kenkunz
Copy link
Author

kenkunz commented Oct 20, 2023

Hi @lforst – thanks for looking into this.

Are you sure this is valid syntax?

Here's the documentation on Type Assertions from the official Typescript docs:
https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#type-assertions

You can also use the angle-bracket syntax (except if the code is in a .tsx file), which is equivalent:

const myCanvas = <HTMLCanvasElement>document.getElementById("main_canvas");

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 20, 2023
@lforst
Copy link
Member

lforst commented Oct 20, 2023

Ah. TIL I guess we need to turn off JSX for the parser then!

cc @Lms24 I don't see any immediate option to do this in magicast, maybe we need to pass in a custom parser?

@kenkunz
Copy link
Author

kenkunz commented Oct 22, 2023

@Lms24 side note – I just saw your Svienna talk on the state of Sentry for Svelte (via Svelte Society YouTube channel)… nice overview… gut getan!

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 22, 2023
@Lms24
Copy link
Member

Lms24 commented Oct 23, 2023

Hi @kenkunz thanks for letting us know about this!

I don't see any immediate option to do this in magicast, maybe we need to pass in a custom parser?

I think this gives us a good reason to switch to recast, providing one of the recast parsers actually supports this syntax. Gonna add this to my to do list for this week.

TIL:
image

This is the AST of <string>x when parsing with the typescript parser

@fromaline
Copy link

+1

I have the same problem right now. Luckily, I can still manually wrap the load functions.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jun 10, 2024
@Lms24
Copy link
Member

Lms24 commented Jun 11, 2024

Hey, sorry that this slipped through 😬 Unfortunately, I'm/we're blocked by a lot of other work at the moment so I can't give you an ETA for fixing this.

For now, @fromaline's suggestion to manually wrap load functions is probably the best workaround if you have to rely on the angle bracket style type assertions. From what I can tell, there's no difference to using as <Type> type assertions which should work fine with our auto instrumentation plugin. But obviously I don't want to force you to change your code just for Sentry. Whatever works for you best!

@fromaline
Copy link

@Lms24, thanks for the explanation! No worries, it's not a deal-breaker.

@Lms24
Copy link
Member

Lms24 commented Mar 4, 2025

It's been a while but I opened #15578 to finally fix this. Sorry for the long wait but better late than never :D

@kenkunz
Copy link
Author

kenkunz commented Mar 4, 2025

Thanks for the update! No worries on the "long wait" – I understand why it wasn't a high priority. Glad you guys kept it in the backlog and finally found a chance to squeeze it in :).

Copy link
Contributor

A PR closing this issue has just been released 🚀

This issue was referenced by PR #15578, which was included in the 9.6.0 release.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Package: sveltekit Issues related to the Sentry SvelteKit SDK Type: Bug
Projects
Archived in project
Archived in project
5 participants