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 service behavior after dependency failure in watch mode #596

Merged
merged 7 commits into from
Dec 9, 2022

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Dec 8, 2022

Previously, if a failure occurred in a persistent (i.e. top-level) service's dependencies, and if we were in watch mode on the 2nd or later iteration, then we completely forgot about the previously running service.

This had two similar effects, both of which could manifest as e.g. "port not available" errors:

  • If the dependency was eventually fixed, we would try to start the service again, even though the previous version was still running.

  • If the user exited wireit with Ctrl-C, we would not shut down the child process because we lost track of it.

This PR fixes these problems by transitioning to a new "started-broken" state when a failure in an already-running service's dependency occurs, which keeps a reference to the child process.

This PR also fixes some failure logging issues, where we would sometimes log the same failure multiple times, or sometimes log a failure at the very end of execution instead of as soon as it actually happens.

Fixes #568
Fixes #540

cc @SanderElias @deebloo @justinfagnani If you'd like to test out this fix early, I've published a pre-release as wireit@0.9.2-pre.1. If you try it, let me know how it goes!

@aomarks aomarks requested a review from justinfagnani December 8, 2022 23:02
@deebloo
Copy link

deebloo commented Dec 9, 2022

@aomarks initial tests look good!

@SanderElias
Copy link

@aomarks This seems to work well. (20 mins into testing. I'll report back if it starts failing later on, but I don't expect so)
However, I noticed that services are now actually watching the things in the files array and get restarted when changes are there. This has changed from before when services didn't do this. I assume it is by intention?

@SanderElias
Copy link

@aomarks It doesn't "crash" my angular services anymore.
However, I still run into an issue. Technically not a WireIt thing, but it is not solvable without a change in it, I'm afraid.
Take an condig like this:

{
  "serve"{
    "command": "ng serve --open --configuration=dev",
    "files": [
      "libs/**"
    ],
    "service": true,
    "dependencies": [
      "build:libs"
    ]
  },
  "build:libs" : {
    "dependencies": [
      "build:lib1",
      "build:lib2"
    ]
  },
  "build:lib1": {
    "command": "ng build --prod --configuration=prod",
    "files": [
      "projects/libs/lib1/**"
    ],
    "output": [
      "libs/lib1"
    ]
  },
  "build:lib2": {
    "command": "ng build --prod --configuration=prod",
    "files": [
      "projects/libs/lib2/**"
    ],
    "output": [
      "libs/lib2"
    ]
  }
}

When one of the libraries fails to build, the 'libs' folder is updated with a faulty lib. When this happens, the ng serve will fail (as expected) but generates a boatload of output, obfuscating the error in the dependency,
I think the solution would be to have an option to 'stop' the service while the deps are rebuilding. Setting the cascade options doesn't seem to have an effect.

(BTW, fixing the error and saving a couple of times again will end up with everything going back to "normal")

@aomarks aomarks merged commit dc20565 into main Dec 9, 2022
@aomarks aomarks deleted the service-bug branch December 9, 2022 16:54
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.

Some services not killed correctly WireIt doesn't kill ng serve properly
4 participants