feat: support for Edge Functions debugging #4615

jackiewmacharia · 2022-05-18T07:51:38Z

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes https://github.com/netlify/pillar-runtime/issues/357
Dependent on: netlify/edge-bundler#31

This PR adds flags to run with netlify dev to extend deno --inspect and deno --inspect-brk for Edge Functions debugging. The --edgeInspect and --edgeInspectBrk have the -e and -E flags respectively as aliases. Each can take a custom host and port that defaults to the default HOST:PORT if none is specified.

To test this out, clone feat/debug-edge-functions-flags branch of edge-bundler; run npm install and npm run build in edge-bundler to set it up; edit CLI package.json to use the cloned version of edge-bundler eg "@netlify/edge-bundler": "file:../edge-bundler" and run some of the below examples in an Edge Functions project.

Examples:

  $ netlify dev --edgeInspect
  $ netlify dev --edgeInspect=127.0.0.1:9229
  $ netlify dev --edgeInspectBrk
  $ netlify dev --edgeInspectBrk=127.0.0.1:9229

For us to review and ship your PR efficiently, please perform the following steps:

Open a bug/issue before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
Read the contribution guidelines 📖. This ensures your code follows our style guide and
passes our tests.
Update or add tests (if any source code was changed or added) 🧪
Update or add documentation (if features were changed or added) 📝
Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

github-actions · 2022-05-18T07:52:26Z

📊 Benchmark results

Comparing with 64fca17

Package size: 287 MB

(no change)

^  287 MB  287 MB  287 MB  287 MB  287 MB  286 MB  286 MB  286 MB  286 MB  286 MB  286 MB  286 MB  287 MB 
│   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T

Legend

T-30 (64fca17): 287 MB
T-29 (4ff745b): 287 MB
T-28 (8de190b): 287 MB
T-27 (328f14d): 287 MB
T-26 (acc745f): 287 MB
T-25 (1a63c0c): 287 MB
T-24 (43c3801): 287 MB
T-23 (4c0c1a8): 287 MB
T-22 (558f402): 287 MB
T-21 (99e3254): 287 MB
T-20 (fc0b984): 287 MB
T-19 (4d7a15b): 287 MB
T-18 (a8bfc66): 287 MB
T-17 (9f04bb0): 287 MB
T-16 (19f6ea5): 287 MB
T-15 (9173d13): 287 MB
T-14 (f8e692f): 287 MB
T-13 (441d7ff): 287 MB
T-12 (5d5aeae): 287 MB
T-11 (9b70a1b): 287 MB
T-10 (ae382d1): 287 MB
T-9 (0009146): 287 MB
T-8 (5f217e5): 287 MB
T-7 (ad23be3): 286 MB
T-6 (7bdf548): 286 MB
T-5 (a139ef1): 286 MB
T-4 (a4c9dbb): 286 MB
T-3 (b6d76e8): 286 MB
T-2 (0845e80): 286 MB
T-1 (3e43d9a): 286 MB
T (current commit): 287 MB

…to feat/debug-edge-functions

…dge bundler

eduardoboucas

Left a few comments, but looking great generally! ✨

eduardoboucas · 2022-05-26T10:06:38Z

src/commands/dev/dev.js

+      new Option(
+        '-E, --edgeInspectBrk [inspectHostPort]',
+        'enable Deno inspect-brk with optional inspectHostPort',
+      ).conflicts('edgeInspect'),


Nice use of .conflicts()! ✨

eduardoboucas · 2022-05-26T10:16:46Z

src/commands/dev/dev.js

+  const getAddress = () => {
+    if (edgeInspect) {
+      // .slice(1) to remove the = in `=127.0.0.1:9229`
+      return typeof edgeInspect === 'string' ? edgeInspect.slice(1) : undefined


I'm confused as to why you need to slice this. Commander is supposed to handle this automatically for you.

I've just pulled your branch, tried using ntl dev --edgeInspect=127.0.0.1:1234 and ntl dev --edgeInspect 127.0.0.1:1234 and in both cases options.edgeInspect has the right contents, with no need to slice.

Am I missing something?

Aha, this was because I was (wrongfully) using the short form that doesn't support the equal sign ending up with edgeInspect: '=127.0.0.1:3030' if ntl dev -e=127.0.0.1:1234 is used.

I've updated the help flag with more examples on how to use the short forms:

'netlify dev -e 127.0.0.1:9229', 'netlify dev -e127.0.0.1:9229', 'netlify dev -E 127.0.0.1:9229', 'netlify dev -E127.0.0.1:9229',

I don't know if this is sufficient though because the error that pops up when -e=127.0.0.1:1234 or -E=127.0.0.1:1234 is ran is what's below from deno. The --help flag suggestion will bring up the above examples though when ran with netlify dev. 🤔

error: Invalid value for '--inspect=<HOST:PORT>...': invalid IP address syntax For more information try --help Caused by: invalid IP address syntax

@eduardoboucas this one is updated and ready

src/commands/dev/dev.js

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

…nd -E

eduardoboucas

Great work! Left a minor, non-blocking comment.

eduardoboucas · 2022-05-30T09:34:24Z

src/commands/dev/dev.js

+      'netlify dev -e',
+      'netlify dev -e 127.0.0.1:9229',
+      'netlify dev -e127.0.0.1:9229',
+      'netlify dev --edgeInspect',
+      'netlify dev --edgeInspect=127.0.0.1:9229',
+      'netlify dev -E',
+      'netlify dev -E 127.0.0.1:9229',
+      'netlify dev -E127.0.0.1:9229',
+      'netlify dev --edgeInspectBrk',
+      'netlify dev --edgeInspectBrk=127.0.0.1:9229',


[dust] I don't think we need to add an example for every single permutation, otherwise we'll end up with a huge list in the help command. How do you feel about keeping just the ones below?

Suggested change

'netlify dev -e',

'netlify dev -e 127.0.0.1:9229',

'netlify dev -e127.0.0.1:9229',

'netlify dev --edgeInspect',

'netlify dev --edgeInspect=127.0.0.1:9229',

'netlify dev -E',

'netlify dev -E 127.0.0.1:9229',

'netlify dev -E127.0.0.1:9229',

'netlify dev --edgeInspectBrk',

'netlify dev --edgeInspectBrk=127.0.0.1:9229',

'netlify dev --edgeInspect',

'netlify dev --edgeInspect=127.0.0.1:9229',

'netlify dev --edgeInspectBrk',

'netlify dev --edgeInspectBrk=127.0.0.1:9229',

I added the short form ones just in case someone uses the = sign with -e or -E and it fails, they see an example of how go use them with the --help flag. This is because something like -e=127.0.0.1:1234 gives us edgeInspect: '=127.0.0.1:3030' because short forms don't support the equal sign causing it to fail.

I understand that, but you could make the same argument for every single parameter we have. The noise of adding every permutation to the help command will make the help command less useful, IMO.

If we're worried that people will misuse the command and get confused about the TypeError: Invalid URL error message (which I agree is not very helpful), we should consider adding validation to the input, so that we throw an error if it's not a valid host/port pair.

You can do this by specifying a custom parser. See https://github.com/tj/commander.js/#custom-option-processing.

@eduardoboucas this is now done, the review also got reset.

…to feat/debug-edge-functions

eduardoboucas

Great work! 🚀

eduardoboucas · 2022-05-31T09:31:15Z

src/commands/dev/dev.js

@@ -27,6 +27,7 @@ const {
  readGraphQLOperationsSourceFile,
 } = require('../../lib/one-graph/cli-netlify-graph')
 const {
+  BANG,


src/commands/dev/dev.js

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

feat: add command to debug Edge Functions using a debugger

62953b5

github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label May 18, 2022

jackiewmacharia and others added 4 commits May 18, 2022 07:52

chore: update contributors field

488e518

chore: update docs

0e5b065

Merge branch 'main' into feat/debug-edge-functions

9460cd0

Merge branch 'feat/debug-edge-functions' of github.com:netlify/cli in…

c846e84

…to feat/debug-edge-functions

jackiewmacharia changed the title ~~feat: add command to debug Edge Functions using a debugger~~ feat: support for Edge Functions debugging May 18, 2022

jackiewmacharia mentioned this pull request May 18, 2022

feat: support for Edge Functions debugging netlify/edge-bundler#31

Merged

2 tasks

jackiewmacharia and others added 6 commits May 19, 2022 14:18

chore: add conflict check and pass single inspectSettings object to e…

1556f2e

…dge bundler

fix: remove the = passed at CLI level

c20f662

Merge branch 'main' into feat/debug-edge-functions

53bd854

Merge branch 'main' into feat/debug-edge-functions

bd907da

Merge branch 'main' into feat/debug-edge-functions

53c2d11

chore: update contributors field

85a1b34

jackiewmacharia marked this pull request as ready for review May 26, 2022 08:10

jackiewmacharia requested a review from a team May 26, 2022 08:10

jackiewmacharia requested review from a team as code owners May 26, 2022 08:10

eduardoboucas reviewed May 26, 2022

View reviewed changes

jackiewmacharia and others added 10 commits May 27, 2022 12:02

chore: expand option definition

5e0b50d

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

chore: use <address> instead

482f047

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

chore: expand option definition

1d74a81

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

Merge branch 'main' into feat/debug-edge-functions

b5fab4a

fix: fix linting

ce40a5b

chore: update docs

1b1cf5f

chore: make address optional

76d54fc

fix: remove .slice operation and update docs with correct use of -e a…

ab2b98a

…nd -E

Merge branch 'main' into feat/debug-edge-functions

5e1d4b4

Merge branch 'main' into feat/debug-edge-functions

2e83ccb

eduardoboucas previously approved these changes May 30, 2022

View reviewed changes

chore: remove additional examples and validate for short form arguments

6682134

jackiewmacharia dismissed eduardoboucas’s stale review via 6682134 May 30, 2022 13:22

jackiewmacharia and others added 4 commits May 30, 2022 13:23

chore: update contributors field

b103202

chore: use flag instead of form

fcd02f3

Merge branch 'feat/debug-edge-functions' of github.com:netlify/cli in…

8751b21

…to feat/debug-edge-functions

Merge branch 'main' into feat/debug-edge-functions

394a9dc

eduardoboucas previously approved these changes May 31, 2022

View reviewed changes

chore: use startsWith for the = sign check

a2cf242

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

jackiewmacharia dismissed eduardoboucas’s stale review via a2cf242 May 31, 2022 09:43

eduardoboucas approved these changes May 31, 2022

View reviewed changes

jackiewmacharia merged commit 0b1d68d into main May 31, 2022

jackiewmacharia deleted the feat/debug-edge-functions branch May 31, 2022 10:11

token-generator-app bot mentioned this pull request May 31, 2022

chore(main): release 10.4.0 #4634

Merged

danez mentioned this pull request Jul 4, 2022

fix: update minimum required version of commander to 9.2.0 #4770

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support for Edge Functions debugging #4615

feat: support for Edge Functions debugging #4615

jackiewmacharia commented May 18, 2022 •

edited

Loading

github-actions bot commented May 18, 2022 •

edited

Loading

eduardoboucas left a comment

eduardoboucas May 26, 2022

eduardoboucas May 26, 2022

jackiewmacharia May 27, 2022

jackiewmacharia May 30, 2022

eduardoboucas left a comment

eduardoboucas May 30, 2022

jackiewmacharia May 30, 2022

eduardoboucas May 30, 2022

eduardoboucas May 30, 2022

jackiewmacharia May 31, 2022

eduardoboucas left a comment

eduardoboucas May 31, 2022

feat: support for Edge Functions debugging #4615

feat: support for Edge Functions debugging #4615

Conversation

jackiewmacharia commented May 18, 2022 • edited Loading

Summary

github-actions bot commented May 18, 2022 • edited Loading

📊 Benchmark results

Package size: 287 MB

eduardoboucas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eduardoboucas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eduardoboucas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackiewmacharia commented May 18, 2022 •

edited

Loading

github-actions bot commented May 18, 2022 •

edited

Loading