feat: hot server reload & debugging over docker #2718

BlairCurrey · 2024-05-10T13:51:48Z

Changes proposed in this pull request

adds hot sever reload on save to auth/backend docker services
exposes debugger with instructions on how to use in most development-environment-agnostic way

Context

fixes #2650
fixes #2672

Checklist

- works to restart servers when changing index.js inside docker (as an intermediate test)

netlify · 2024-05-10T13:52:04Z

✅ Deploy Preview for brilliant-pasca-3e80ec canceled.

Name	Link
🔨 Latest commit	`38ad16e`
🔍 Latest deploy log	https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/664b5f2290bd0a0008f776e1

BlairCurrey · 2024-05-10T14:05:33Z

localenv/README.md

+### Debugging
+
+Debuggers for the services are exposed on the following ports:
+
+| IP and Port    | Services                |
+| -------------- | ----------------------- |
+| 127.0.0.1:9229 | Cloud Nine Backend      |
+| 127.0.0.1:9230 | Cloud Nine Auth         |
+| 127.0.0.1:9231 | Happy Life Bank Backend |
+| 127.0.0.1:9232 | Happy Life Bank Auth    |
+
+To use these debuggers with a chromium browser:
+
+- go to `chrome://inspect` in chromium browser
+- Click "Configure" add the IP addresses and ports detailed above
+- start docker containers
+- click "inspect" on the service you want to debug to open the chromium debugger
+
+You can either trigger the debugger by adding `debugger` statements in code and restarting, or by adding breakpoints directly in the chromium debugger after starting the docker containers.
+
+For more ways to connect to these debuggers, see the Node docs for debugging: https://nodejs.org/en/learn/getting-started/debugging
+


I wanted to provide specific instructions for a basic way to use the debugger despite the fact that everyone is liable to have a different setup (vscode, jetbrains, text editor, etc.). Thus I provided instructions for setting up the chromium debugging client and left a link to the node docs if people want to tune it more for their own setup.

Of course this does require a chromium browser, but it's still the most developer-environment-agnostic option. Note that it's not possible to use safari, firefox, or any non-chromium browser because node uses the v8 engine, and those browsers do not.

Please let me know if you can follow these instructions to start using the debugger successfully.

BlairCurrey · 2024-05-10T14:10:35Z

localenv/mock-account-servicing-entity/app/entry.server.tsx

@@ -21,7 +21,7 @@ async function callWithRetry(fn: () => any, depth = 0): Promise<void> {
  try {
    return await fn()
  } catch (e) {
-    if (depth > 7) {
+    if (depth > 10) {


the new startup command in the dockerfile takes longer because we have to rebuild and copy over gql/openapi schemas. 9 was all that was required but figured I'd bump it to 10 for some headroom.

Do we still need to change this with ts-node-dev?

no we dont, changed back to 7

BlairCurrey · 2024-05-10T14:14:11Z

packages/auth/package.json

@@ -18,7 +18,8 @@
    "prepack": "pnpm build",
    "postinstall": "pnpm copy-op-schemas",
    "copy-op-schemas": "cp ./node_modules/@interledger/open-payments/dist/openapi/specs/auth-server.yaml ./node_modules/@interledger/open-payments/dist/openapi/specs/schemas.yaml ./src/openapi/specs/",
-    "copy-files": "cp src/graphql/schema.graphql dist/graphql/ && cp -r ./src/openapi ./dist/"
+    "copy-files": "cp src/graphql/schema.graphql dist/graphql/ && cp -r ./src/openapi ./dist/",
+    "dev": "nodemon --watch src --ext ts --exec \"tsc && pnpm copy-files && node --inspect=0.0.0.0:9229 dist/index.js\""


This is run from the dev dockerfile. In plain english, it is:

rebuilding, copying over the gql/openapi schema files, and starting the sever with the debugger. Then on the docker container's src file change, it is repeating this rebuild/copy/server start. The docker container's /src dir mirrors the host machine's /src dir via a bind mount. Thus saving changes on the host machine triggers the server restart in docker.

Alternatives I considered included:

using ts-node instead of explicit build step. Never really liked the idea but its the more common use-case with nodemon. Tried it briefly but ran into some typescript errors and didn't pursue it further.

Docker Compose Watch. This could be used to varying extents. Effectively replacing nodemon, or replacing nodemon and bind mount. However we dont really take advantage of the advantages over bind mounts listed in the docs and it would create more overheard on restart (restarts entire container instead of just server process).

I had a sort of vague idea about doing a bind mount on /dist instead and adding something to watch on the hostmachine (tacked onto localenv:compose and localenv:compose:psql probably), but then we'd still need to watch the /dist in docker as well. Overall I felt like this was more complicated.

BlairCurrey · 2024-05-10T14:27:34Z

localenv/cloud-nine-wallet/docker-compose.yml

@@ -110,7 +122,7 @@ services:
    image: rafiki-frontend
    build:
      context: ../..
-      dockerfile: ./packages/frontend/devDockerfile
+      dockerfile: ./packages/frontend/Dockerfile.dev


went ahead and renamed this for consistency. I went with Dockerfile.dev because that seemed to be a more standard convention from examples I saw a variety of places. VSCode also recognizes its a dockerfile from name alone, whereas it did not with devDockerfile.

BlairCurrey · 2024-05-10T14:52:51Z

It's not a factor in these changes, but I noticed we have docker-compose in our services, like rafiki/packages/backend/docker-compose.yml. Why?

As far as I can tell they are unused. Do we need them? Should I get rid of them?

BlairCurrey · 2024-05-10T17:50:36Z

I considered adding a CI step for checking that frontend starts OK with the production image. This is not trivial to setup since frontend depends on quite a few things (kratos, backend db, etc.). It would also be completely redundant if we
add any sort of frontend e2e test as part of this issue, so I figured we could defer to that: #2627

mkurapov · 2024-05-14T13:28:10Z

using ts-node instead of explicit build step. Never really liked the idea but its the more common use-case with nodemon. Tried it briefly but ran into some typescript errors and didn't pursue it further.

@BlairCurrey curious to hear your thoughts on debugging the complied JS files or the TS files directly.

I tried using ts-node-dev in this commit, and it ended up working quite well with the following VS code launch.json for debugging cloud-nine-backend (with the remote root pointing to the volume you expose):

    {
      "name": "Attach to docker (cloud-nine-backend)",
      "type": "node",
      "request": "attach",
      "port": 9229,
      "address": "localhost",
      "localRoot": "${workspaceFolder}",
      "remoteRoot": "/home/rafiki/",
      "restart": true
    },

Let me know what feels better/what issues you had when trying TS, but it was quite nice debugging the files we end up writing.

BlairCurrey · 2024-05-14T14:53:50Z

using ts-node instead of explicit build step. Never really liked the idea but its the more common use-case with nodemon. Tried it briefly but ran into some typescript errors and didn't pursue it further.

@BlairCurrey curious to hear your thoughts on debugging the complied JS files or the TS files directly.

I tried using ts-node-dev in this commit, and it ended up working quite well with the following VS code launch.json for debugging cloud-nine-backend (with the remote root pointing to the volume you expose):
    {
      "name": "Attach to docker (cloud-nine-backend)",
      "type": "node",
      "request": "attach",
      "port": 9229,
      "address": "localhost",
      "localRoot": "${workspaceFolder}",
      "remoteRoot": "/home/rafiki/",
      "restart": true
    },
Let me know what feels better/what issues you had when trying TS, but it was quite nice debugging the files we end up writing.

Thanks for sharing this - seems promising. I like that we dont have to copy files every change (since they arent being wiped out by building).

mkurapov · 2024-05-16T08:37:08Z

localenv/README.md

@@ -139,6 +139,28 @@ Note that you have to go through an additional "login" step by providing you IPv

 After stopping the script it is necessary to clear the environment using the command described in [Shutting down](#Shutting-down). This is necessary as on a new run of the scripts (with autopeering or not) the wallet address url will differ.

+### Debugging


Maybe we can add a section with the launch.json configuration for VSCode as well?

Also, did you want to keep it just in this readme, or should we also add it to the documentation site?

added the vscode section and added to the documentation site: 02a5e6b

mkurapov · 2024-05-16T08:38:11Z

localenv/mock-account-servicing-entity/app/entry.server.tsx

@@ -21,7 +21,7 @@ async function callWithRetry(fn: () => any, depth = 0): Promise<void> {
  try {
    return await fn()
  } catch (e) {
-    if (depth > 7) {
+    if (depth > 10) {


Do we still need to change this with ts-node-dev?

mkurapov

v minor comments

localenv/README.md

mkurapov · 2024-05-16T22:19:22Z

packages/auth/package.json

@@ -66,6 +68,7 @@
    "jest-openapi": "^0.14.2",
    "nock": "^13.5.4",
    "node-mocks-http": "^1.14.1",
+    "nodemon": "^3.1.0",


Suggested change

"nodemon": "^3.1.0",

good catch, committed locally and updated the lockfile

Co-authored-by: Max Kurapov <max@interledger.org>

BlairCurrey added 6 commits May 7, 2024 11:20

refactor: auth, backend dockerfiles into dev/prod

649ff1d

feat: start backend, auth with nodemon

e2c6a29

- works to restart servers when changing index.js inside docker (as an intermediate test)

feat: reload auth, backend on save

1fba772

feat: hot reload of auth/backend

e59e8b5

feat: add debugger w/ docs

9336e1b

Merge branch 'main' into bc/2650/service-reload-over-docker

6389803

github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. pkg: frontend Changes in the frontend package. pkg: auth Changes in the GNAP auth package. type: localenv Local playground pkg: mock-ase labels May 10, 2024

BlairCurrey changed the title ~~Bc/2650/service reload over docker~~ feat: hot server reload & debugging over docker May 10, 2024

BlairCurrey commented May 10, 2024

View reviewed changes

refactor: rm unecessary EXPOSE from dockerfiles

01d2d09

BlairCurrey commented May 10, 2024

View reviewed changes

fix(ci): use new dockerfile name

10e8019

github-actions bot added the type: ci Changes to the CI label May 10, 2024

BlairCurrey marked this pull request as ready for review May 10, 2024 15:28

BlairCurrey requested review from mkurapov, JoblersTune, sabineschaller and njlie May 10, 2024 15:28

refactor: edit debugger instructions for clarity

263e3ce

Merge branch 'main' into bc/2650/service-reload-over-docker

ae28599

feat: use debugger on ts files, replace nodemon with ts-node-dev

27e0fff

mkurapov reviewed May 16, 2024

View reviewed changes

chore: change retry rate, add ts-node transpiler to tsconfig

9c58f51

github-actions bot removed the pkg: mock-ase label May 16, 2024

feat: add debugging instructions to docs

02a5e6b

github-actions bot added pkg: documentation Changes in the documentation package. type: documentation Improvements or additions to documentation labels May 16, 2024

chore: format

b9c415c

BlairCurrey requested a review from mkurapov May 16, 2024 18:38

mkurapov reviewed May 16, 2024

View reviewed changes

BlairCurrey and others added 4 commits May 20, 2024 09:55

Update localenv/README.md

07d81a1

Co-authored-by: Max Kurapov <max@interledger.org>

Update localenv/README.md

66663a9

Co-authored-by: Max Kurapov <max@interledger.org>

chore: rm nodemon

2de0f80

chore: format

38ad16e

BlairCurrey requested a review from mkurapov May 20, 2024 15:01

mkurapov approved these changes May 21, 2024

View reviewed changes

BlairCurrey merged commit e8d57b7 into main May 21, 2024
42 checks passed

BlairCurrey deleted the bc/2650/service-reload-over-docker branch May 21, 2024 16:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: hot server reload & debugging over docker #2718

feat: hot server reload & debugging over docker #2718

BlairCurrey commented May 10, 2024 •

edited

netlify bot commented May 10, 2024 •

edited

BlairCurrey May 10, 2024 •

edited

BlairCurrey May 10, 2024

BlairCurrey May 10, 2024 •

edited

mkurapov May 16, 2024

BlairCurrey May 16, 2024

BlairCurrey May 10, 2024 •

edited

BlairCurrey May 10, 2024 •

edited

BlairCurrey May 10, 2024 •

edited

BlairCurrey commented May 10, 2024

BlairCurrey commented May 10, 2024 •

edited

mkurapov commented May 14, 2024 •

edited

BlairCurrey commented May 14, 2024 •

edited

mkurapov May 16, 2024

BlairCurrey May 16, 2024 •

edited

mkurapov May 16, 2024

mkurapov left a comment

mkurapov May 16, 2024

BlairCurrey May 20, 2024 •

edited

		@@ -139,6 +139,28 @@ Note that you have to go through an additional "login" step by providing you IPv

		After stopping the script it is necessary to clear the environment using the command described in [Shutting down](#Shutting-down). This is necessary as on a new run of the scripts (with autopeering or not) the wallet address url will differ.

		### Debugging

feat: hot server reload & debugging over docker #2718

feat: hot server reload & debugging over docker #2718

Conversation

BlairCurrey commented May 10, 2024 • edited

Changes proposed in this pull request

Context

Checklist

netlify bot commented May 10, 2024 • edited

✅ Deploy Preview for brilliant-pasca-3e80ec canceled.

BlairCurrey May 10, 2024 • edited

Choose a reason for hiding this comment

BlairCurrey May 10, 2024

Choose a reason for hiding this comment

BlairCurrey May 10, 2024 • edited

Choose a reason for hiding this comment

mkurapov May 16, 2024

Choose a reason for hiding this comment

BlairCurrey May 16, 2024

Choose a reason for hiding this comment

BlairCurrey May 10, 2024 • edited

Choose a reason for hiding this comment

BlairCurrey May 10, 2024 • edited

Choose a reason for hiding this comment

BlairCurrey May 10, 2024 • edited

Choose a reason for hiding this comment

BlairCurrey commented May 10, 2024

BlairCurrey commented May 10, 2024 • edited

mkurapov commented May 14, 2024 • edited

BlairCurrey commented May 14, 2024 • edited

mkurapov May 16, 2024

Choose a reason for hiding this comment

BlairCurrey May 16, 2024 • edited

Choose a reason for hiding this comment

mkurapov May 16, 2024

Choose a reason for hiding this comment

mkurapov left a comment

Choose a reason for hiding this comment

mkurapov May 16, 2024

Choose a reason for hiding this comment

BlairCurrey May 20, 2024 • edited

Choose a reason for hiding this comment

BlairCurrey commented May 10, 2024 •

edited

netlify bot commented May 10, 2024 •

edited

BlairCurrey May 10, 2024 •

edited

BlairCurrey May 10, 2024 •

edited

BlairCurrey May 10, 2024 •

edited

BlairCurrey May 10, 2024 •

edited

BlairCurrey May 10, 2024 •

edited

BlairCurrey commented May 10, 2024 •

edited

mkurapov commented May 14, 2024 •

edited

BlairCurrey commented May 14, 2024 •

edited

BlairCurrey May 16, 2024 •

edited

BlairCurrey May 20, 2024 •

edited