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

fix: update source-map-support to get correct filepath #521

Merged
merged 1 commit into from
Jun 8, 2022

Conversation

vigneshshanmugam
Copy link
Member

@vigneshshanmugam vigneshshanmugam commented Jun 3, 2022

How to test

  1. Build the library npm run build
  2. Install the agent globally npm i -g .
  3. Scaffolding a new project using npx elastic-synthetics init test-dir
  4. Run the push command inside directory - cd test-dir && npm run push
  5. Extra tests can be performed using cjs, mjs and ESM formats.

@apmmachine
Copy link
Collaborator

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-06-03T23:28:35.267+0000

  • Duration: 16 min 44 sec

Test stats 🧪

Test Results
Failed 0
Passed 165
Skipped 2
Total 167

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

Copy link
Contributor

@lucasfcosta lucasfcosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just attempted to test this PR E2E and still had an error.

Here's what I've attempted:

  1. Checkout this branch & nvm use
  2. Remove my node_modules folder and run npm cache clean --force to ensure I'd have the correct dependencies
  3. Run npm ci to install the exact deps in package-lock.json
  4. Run npm run build to get a built version in dist
  5. Run the built CLI with node ./dist/cli.js init test-dir to create a project
  6. cd into test-dir and try to run push
    node ../dist/cli.js push . --auth "TOKEN_HERE" --url http://127.0.0.1:5602/ywn --project test-proj --delete
    

That process resulted in:

➜ node ../dist/cli.js push . --auth "TOKEN_HERE" --url http://127.0.0.1:5602/ywn --project test-12 --delete
> preparing all monitors
✘ [ERROR] [plugin esbuild-multiasset-plugin] ENOENT: no such file or directory, open '/Users/lucasfcosta/Repositories/synthetics/test-dir/journeys/file:/Users/lucasfcosta/Repositories/synthetics/test-dir/journeys/example.journey.ts'

  This error came from the "onLoad" callback registered here:

    ../src/push/plugin.ts:106:12:
      106 │       build.onLoad({ filter: /.*?/, namespace: 'asset' }, async args => {
          ╵             ~~~~~~

    at setup (/Users/lucasfcosta/Repositories/synthetics/src/push/plugin.ts:106:13)
    at handlePlugins (/Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:839:23)
    at Object.buildOrServe (/Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:1133:7)
    at /Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:2074:17
    at new Promise (<anonymous>)
    at Object.build (/Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:2073:14)
    at Object.build (/Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:1923:51)
    at Bundler.prepare (/Users/lucasfcosta/Repositories/synthetics/src/push/bundler.ts:56:36)
    at Bundler.build (/Users/lucasfcosta/Repositories/synthetics/src/push/bundler.ts:84:16)

Error: Build failed with 1 error:
error: ENOENT: no such file or directory, open '/Users/lucasfcosta/Repositories/synthetics/test-dir/journeys/file:/Users/lucasfcosta/Repositories/synthetics/test-dir/journeys/example.journey.ts'
    at failureErrorWithLog (/Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:1599:15)
    at /Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:1245:28
    at runOnEndCallbacks (/Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:1030:63)
    at buildResponseToResult (/Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:1243:7)
    at /Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:1352:14
    at /Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:662:9
    at handleIncomingPacket (/Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:759:9)
    at Socket.readFromStdout (/Users/lucasfcosta/Repositories/synthetics/node_modules/esbuild/lib/main.js:629:7)
    at Socket.emit (node:events:527:28)
    at Socket.emit (node:domain:475:12) {
  errors: [
    {
      detail: [Error],
      location: null,
      notes: [Array],
      pluginName: 'esbuild-multiasset-plugin',
      text: "ENOENT: no such file or directory, open '/Users/lucasfcosta/Repositories/synthetics/test-dir/journeys/file:/Users/lucasfcosta/Repositories/synthetics/test-dir/journeys/example.journey.ts'"
    }
  ],
  warnings: []
}
> creating all monitors
✓ Pushed

@vigneshshanmugam btw if you're going to be off for a couple of days I'd be happy to pick up this one, just lmk. Otherwise, I'll come back to this one by Monday.

@lucasfcosta
Copy link
Contributor

Okay, so after an hour of debugging I've realised this was all my fault during the testing. Here's a funny story:

So, the way we get file's locations is through overriding Error.prepareStackTrace. That's the function which generates frames for the stack trace (see obscure docs here).

synthetics/src/helpers.ts

Lines 297 to 323 in 717303f

export function wrapFnWithLocation<A extends unknown[], R>(
func: (location: Location, ...args: A) => R
): (...args: A) => R {
return (...args) => {
const _prepareStackTrace = Error.prepareStackTrace;
Error.prepareStackTrace = (_, stackFrames) => {
// Deafult CallSite would not map to the original transpiled source
// from ts-node properly, So we wrap it with the library that knows
// how to retrive those source-map for the transpiled code
const frame: NodeJS.CallSite = sourceMapSupport.wrapCallSite(
stackFrames[1]
);
return {
file: frame.getFileName(),
line: frame.getLineNumber(),
column: frame.getColumnNumber(),
};
};
Error.stackTraceLimit = 2;
const obj: { stack: Location } = {} as any;
Error.captureStackTrace(obj);
const location = obj.stack;
Error.stackTraceLimit = dstackTraceLimit;
Error.prepareStackTrace = _prepareStackTrace;
return func(location, ...args);
};
}

Within that function, we wrap the file's frame into a call to the third-party module for handling source-maps. Thanks to that call, when we run frame.getFileName we get the actual filename.

synthetics/src/helpers.ts

Lines 306 to 308 in 717303f

const frame: NodeJS.CallSite = sourceMapSupport.wrapCallSite(
stackFrames[1]
);

Then, we use captureStackTrace to get the location object we need and pass it to the function we'd like to wrap

synthetics/src/helpers.ts

Lines 317 to 321 in 717303f

Error.captureStackTrace(obj);
const location = obj.stack;
Error.stackTraceLimit = dstackTraceLimit;
Error.prepareStackTrace = _prepareStackTrace;
return func(location, ...args);

Now, the way the filename gets passed to journey is through wrapping the journey function into the function I described above.

export const journey = wrapFnWithLocation(
(
location: Location,
options: JourneyOptions | string,
callback: JourneyCallback
) => {
log(`register journey: ${JSON.stringify(options)}`);
if (typeof options === 'string') {
options = { name: options, id: options };
}
const j = new Journey(options, callback, location);
runner.addJourney(j);
return j;
}
);

The problem here is that the journey we require within the project created with init is the journey function from the package in NPM!

Therefore, it uses the old version of the cspotcode/node-source-map-support package, the one which doesn't have the fix!

To properly test this fix, I needed the new journey (from the built package). So I had to purge node_modules, and update package.json to point to my locally built package:

{
  // ...
  "dependencies": {
    "@elastic/synthetics": "/Users/lucasfcosta/Repositories/synthetics/dist/index.js"
  }
}

Copy link
Contributor

@lucasfcosta lucasfcosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewvc I think you're gonna find this one funny 😄

@lucasfcosta lucasfcosta merged commit 7edf576 into elastic:main Jun 8, 2022
@vigneshshanmugam vigneshshanmugam deleted the fix-source-map branch June 14, 2022 04:52
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

Successfully merging this pull request may close these issues.

source location for journey files has incorrect filepath
3 participants