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 missing "ps" terminal command on app crash/exit #944

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

dimon222
Copy link
Contributor

@dimon222 dimon222 commented Aug 6, 2024

Fixes errors with invocations of "ps" terminal command...
Current master/latest contains some code/libraries that invoke "ps" command under certain circumstances (perhaps specific architectures or such).

Example of error this meant to solve:

[backend] 2024-08-06T01:06:23.929688Z  INFO sea_orm_migration::migrator: Applying all pending migrations
[backend] 2024-08-06T01:06:23.935904Z  INFO sea_orm_migration::migrator: No pending migrations
[backend] thread 'main' panicked at apps/backend/src/main.rs:159:28:
[backend] called `Result::unwrap()` on an `Err` value: ParseError(())
[backend] BACKEND_PORT=5000 /usr/local/bin/ryot exited with code 101
node:events:492
      throw er; // Unhandled 'error' event
      ^

Error: spawn ps ENOENT
    at ChildProcess._handle.onexit (node:internal/child_process:286:19)
    at onErrorNT (node:internal/child_process:484:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
Emitted 'error' event on ChildProcess instance at:
    at onErrorNT (node:internal/child_process:484:16)
  errno: -2,
  code: 'ENOENT',
  syscall: 'spawn ps',
  path: 'ps',
  spawnargs: [ '-o', 'pid', '--no-headers', '--ppid', 17 ]
}

Node.js v20.10.0

PS: it may not solve the problem of the core reason of why app itself crashed in my particular example, but it will allow app to handle shutdown gracefully in case of shutdown/crash flow.

Fixes errors with invocations of "ps" terminal command
@CLAassistant
Copy link

CLAassistant commented Aug 6, 2024

CLA assistant check
All committers have signed the CLA.

@dimon222 dimon222 changed the title Fix missing "ps" terminal command Fix missing "ps" terminal command on app crash/exit Aug 6, 2024
@IgnisDa
Copy link
Owner

IgnisDa commented Aug 6, 2024

Can you tell me how to reproduce the issue? The particular error you are getting comes from concurrently killing the frontend if the backend dies. So that error is expected.

@dimon222
Copy link
Contributor Author

dimon222 commented Aug 6, 2024

Can you tell me how to reproduce the issue? The particular error you are getting comes from concurrently killing the frontend if the backend dies. So that error is expected.

In my case I caused the state when app wasn't able to launch like specified TZ with typo. I suspect it would throw this error always for any state when backend doesn't start or when it crashes and based on error it invokes "ps" command but it doesn't exist (not installed in os). With ps installed the shutdown hook executes gracefully without this extra error that likely will mislead end user.

@IgnisDa
Copy link
Owner

IgnisDa commented Aug 6, 2024

Makes sense. I will merge this once #939 is merged since that will introduce conflicts.

@IgnisDa IgnisDa merged commit a451df4 into IgnisDa:main Aug 8, 2024
1 check passed
@IgnisDa
Copy link
Owner

IgnisDa commented Aug 8, 2024

Thanks for the contribution!

@dimon222 dimon222 deleted the patch-1 branch August 8, 2024 02:10
vnghia added a commit to vnghia/ryot that referenced this pull request Aug 10, 2024
IgnisDa added a commit that referenced this pull request Aug 11, 2024
* ci: add cross-rs build

* add missing env

* add env file

* add build frontend

* use moonrepo setup toolchain

* use moon docker

* delete docker setup

* add artifact path

* download templates

* add path

* add path

* add ls

* add templates

* use abs

* typo

* rename artifact

* add build docker

* env permission

* add ghcr

* add libssl

* openssl

* remove curl

* add tar

* add path

* cd

* --strip-components

* cd

* apps/frontend/

* build frontend twice

* remove debug assertion

* build on host

* sudo

* arm64

* arm64

* libssl

* rustup

* ci: add new token

* set env

* Revert "build on host"

This reverts commit a47df4e.

* install libssl1.1 for arm64

* lint

* typo

* typo

* sh not bash

* y

* docker tag

* cache when not in a pull request

* delete old files

* enable

* passenv

* re-add the change from #944

* ci: change name of file

* ci: decide whether to run workflow in PR

* ci: test commit

Run CI.

* ci: test commit

Run CI.

* ci: Run CI

* ci: dump contents [Run CI]

* ci: add more logging [RUN CI]

* ci: select correct commit message [Run CI]

* ci: remove useless comment

* ci: add caching to docker action

* ci: test whole commit message if workflow should run

Run CI.

* also push to docker hub

* typo [Run CI]

* ci: enable dockerhub only for main releases

* ci: remove useless echo statements

[Run CI]

* use repository_owner [Run CI]

* ci: setup caching for building frontend

* ci: build only in release mode

[Run CI]

* ci: add new steps

* ci: change name of job

[Run CI]

* ci: change name of output

* ci: add step to create release

* ci: do not build frontend in gh action

* ci: do not build transactional in dockerfile

[Run CI]

* ci: change name of step

---------

Co-authored-by: Diptesh Choudhuri <[email protected]>
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.

3 participants