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

[Feature request] Support breakpoints on cloud function emulators #165

Open
ciriousjoker opened this issue Oct 23, 2023 · 7 comments
Open
Labels
enhancement New feature or request

Comments

@ciriousjoker
Copy link

ciriousjoker commented Oct 23, 2023

I added this in our project by doing the following:

  1. Add "sourcemap": true to the esbuildOptions.
  2. Add --inspect-functions to the emulators:start command

Sample launch.json

{
  "version": "0.2.0",
  "configurations": [
    {
      "name": "Attach to firebase emulators",
      "port": 9229,
      "request": "attach",
      "skipFiles": ["<node_internals>/**"],
      "type": "node"
    }
  ]
}
@ciriousjoker
Copy link
Author

@simondotm
Copy link
Owner

Been busy with other work lately, will look at this asap, sounds great though

@simondotm simondotm added the enhancement New feature or request label Nov 5, 2023
@simondotm
Copy link
Owner

simondotm commented Nov 6, 2023

This is a nice feature request. Some discussion topics on it:

I would like to propose that further additions are needed before we could release this feature:

  • e2e tests should verify that build output does indeed generate *.js.map files in dist
  • Since this feature changes the project.json schema, we should update the plugin migration tool to add these changes to existing plugin users workspace firebase projects.
  • Discuss if map files should be deployed or ignored
    • If they are deployed, we'd need to document how plugin users can use them such as described here
    • If not, we'd need to add ignore rules to function configs for *.map files
  • As you already mentioned, we should also document these new enhancements.

@timofei-iatsenko
Copy link

Discuss if map files should be deployed or ignored
If they are deployed, we'd need to document how plugin users can use them such as described here
If not, we'd need to add ignore rules to function configs for *.map files

add my 5 cents here. The sourcemaps should be definitely deployed by default. Since code is bundled finding a real file causing an issue would be hard without sourcemaps. Default code generator should insert import 'source-map-support/register'; with a comment why it used instead of "--enable-source-maps" native node flag.

@simondotm
Copy link
Owner

@thekip thanks for your comment. I broadly agree.
One concern I have though is runtime performance for functions. source map files can be large, and loading them wont be cheap, do we know if they are only loaded when a crash occurs?

@simondotm
Copy link
Owner

simondotm commented Apr 6, 2024

@thekip

...with a comment why it used instead of "--enable-source-maps" native node flag.

Doing a bit of research, it seems as though using node's --enable-source-maps option would be the easiest implementation, since it brings native node support for source maps, and we could just define:

NODE_OPTIONS=–enable-source-maps

in our .env or .env.local file I believe. This wouldn't require any additional workspace dependencies for source-map-support nor any wrangling with esbuild banners to import it at the top of the bundle. It might be good enough for most use cases.

However, there's a very interesting discussion here indicating that the node implementation is extremely slow with larger sourcemap files (like the ones we'd get with esbuild bundles), which seems to add risk of performance penalties in production cloud functions - not ideal given that CPU time is billed.

It does seem that performance costs for either node native or source-map-support only seem to be incurred when an Error's stack trace is accessed, which is reassuring.

One contributor has benchmarked various source map solutions here, with node native support not faring too well in v16-v17, and also flagging incorrect stack traces which is not encouraging. There's no data in the benchmarks for v18+.

Some of these concerns may be more related to how big or minified the source map files are however, so on a related note, this comment is interesting also - using an esbuild plugin to exclude source maps for bundled external deps - though to be fair, for Firebase functions generated with this plugin, we already exclude bundling of external dependencies.

@simondotm
Copy link
Owner

I think I'm going to proceed carefully with this feature, and:

  1. Have esbuild always emit source maps, and have those deployed
  2. Enable the --inspect-functions support in the serve target
  3. Set NODE_OPTIONS=–enable-source-maps in the .env.local file

This at least sets a foundation for debugging functions locally.

We can then see what is the best way forward for supporting source maps in deployed functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants