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

Bug: inline sourcemaps are broken #2680

Closed
rob3c opened this issue Feb 25, 2021 · 6 comments · Fixed by #2837
Closed

Bug: inline sourcemaps are broken #2680

rob3c opened this issue Feb 25, 2021 · 6 comments · Fixed by #2837
Labels
bug This issue is a bug. effort/large Large work item – several weeks of effort p1

Comments

@rob3c
Copy link

rob3c commented Feb 25, 2021

The AWS CDK inline sourcemaps are broken. This is because sourcemaps are based on the original typescript source files, while the inlined typescript source code has been stripped down in some way so they're shorter and don't align with sourcemap locations.

This mismatch prevents debuggers from stepping through source correctly. It also causes wrong code excerpts to appear in CDK CLI error output. Both of these problems can make it much harder to debug a CDK app than it needs to be. That's too bad, because the CDK is a wonderful tool, and simpler debugging would make it even better!

Reproduction Steps

There are different ways to show the problem for a given CDK version. I've checked 1.91.0 (currently the latest 1.x) as well as the earlier 1.85.0 release. I'll use the vscode debugger below with screenshots, which seems simplest here.

(Note that I created the annotations in the screenshots before writing this text, and some imprecisely refer to the original source as 'correct' and the inline source as 'wrong' in terms of sourcemap locations. Of course, what really matters is that sourcemap locations align with the inlined code, not that they necessarily match the original files.)

Repro 1

  1. Find the source for the version we're using as a reference. For example, use the github branch picker online, or clone locally:
git clone https://github.com/aws/aws-cdk.git
git checkout tags/v1.91.0
# No need to waste time building here, we just want to see source files
  1. Initialize and build a minimal CDK app to debug, then open in vscode:
cdk init app --language typescript
npm run build
code .
  1. Switch to the debugger tab in vscode, and create a new nodejs launch config.
    3_cdkbug_launch_config

  2. Set a breakpoint at the new cdk.App call:
    4_cdkbug_breakpoint_at_new_app_call

  3. Step into the cdk source for the App constructor. This is really the inline typescript source at the end of the js file. Notice that the debugger has placed us at the end of the file. That's because the 101:5 location it tries to navigate to is based on the original source that has 147 lines, but the debugger uses the inline source that only has 86 lines:
    5_cdkbug_after_stepping_in_stack_correct_but_wrong_file_location

  4. Here's a screenshot from the original v1.91.0 app.ts source file, confirming that the sourcemap line:column indicator in the stack (101:5) matches it:
    6_cdkbug_correct_location_in_original_ts_source

Repro 2

As a further example, I added this code to force a cdk synth CLI error. The fact that this causes an error is surprising to me, since it seems I should only need to create a single HostedZone instance once using fromLookup that I can pass to multiple stacks via props. Anyway, I'll probably create a separate issue for it. Debugging it is what prompted reporting this issue.

  1. Here's the main source change:
    1_cdk_synth_error_app_update

  2. Here's the cdk synth CLI output stack trace with incorrect source code excerpt. In the screenshot, notice all stack locations match the original typescript source but not the inline source:
    2_cdk_synth_error_stack_and_bad_src_excerpt

  3. Here's the reported line 191 from the original source, which is the correct source of the thrown error:
    3_cdk_synth_error_line_191_matches_orig_src

4.a And here's where the code excerpt text is from in the original source at line 369:
4_cdk_synth_error_bad_src_excerpt_from_orig_line_369

What did you expect to happen?

I expect the sourcemap locations to match the inline source code exactly.

What actually happened?

The sourcemaps do not match the inline source code, which causes two problems:

  1. The debugger cannot step through the source correctly, causing a debugging nightmare lol.
  2. Source code excerpts in stack trace errors during CDK CLI operations are wrong and misleading.

Environment

  • CDK CLI Version : 1.91.0 (doesn't matter but also 1.85.0)
  • Framework Version: 1.91.0 (also earlier in at least 1.85.0)
  • Node.js Version: 14.15.4 (doesn't matter but also 12.18.3)
  • OS : Windows 10 (doesn't matter but also macOS)
  • Language (Version): <!-- TypeScript 3.9.9 (doesn't matter but also 3.9.7)

Other

It appears that there's a code reduction step in the build process that's stripping down the source that ultimately gets inserted inline in the js files. Unfortunately, it seems the sourcemaps are created before this happens, so they match the original source instead of the inlined source. Hopefully, there may be a simple fix by just reordering some build steps, but maybe that's just wishful thinking :-)


This is 🐛 Bug Report

@MrArnoldPalmer
Copy link
Contributor

Adding @rix0rrr cause I know he uses the vs code debugger and may have a better idea of whats going on here.

@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 2, 2021

Don't use it a lot :)

I think what's happening here is that the source maps get generated by TSC based on the original TypeScript source it sees, but then jsii applies some additional transforms to the JavaScript source which cause the sourcemaps to desync.

assembler.customTransformers,

This is only a guess, but that is my guess. We'd estimate this by looking at the displacements of where VSCode thinks the source is, and then estimating the number of characters that the (deprecated) and runtime-info-related generated code occupy and seeing if they are about the same size.

This should probably be bumped to the jsii repository, and the solution is probably of the form of doing the transforms earlier, or forcing the source maps to be recalculated. @RomainMuller wdyt?

@rob3c
Copy link
Author

rob3c commented Mar 5, 2021

Thanks @rix0rrr that confirms my suspicions. I appreciate your quickly identifying jsii transformations as a likely cause of the problem, and sharing a relevant source location is a nice bonus!

Should I also create an issue there that links here, or is someone from the CDK team already planning to do it?

I see there's a jsii issue label here that can be applied for cases like this. There are several issues labeled with it that remain open (my project mgmt pref too), so I won't try to close this issue yet.

@rix0rrr rix0rrr removed their assignment Mar 8, 2021
@MrArnoldPalmer
Copy link
Contributor

@rob3c go ahead and create an issue in jsii and close this in favor of that when done.

@RomainMuller RomainMuller transferred this issue from aws/aws-cdk Mar 12, 2021
@RomainMuller RomainMuller changed the title AWS CDK inline sourcemaps are broken Bug: inline sourcemaps are broken Mar 12, 2021
@RomainMuller
Copy link
Contributor

Transferred to the jsii repository.

@RomainMuller RomainMuller added bug This issue is a bug. effort/large Large work item – several weeks of effort p1 labels Mar 12, 2021
lzhoucs added a commit to lzhoucs/jsii that referenced this issue May 13, 2021
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. effort/large Large work item – several weeks of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants