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.

Sign up for GitHub

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

Jump to bottom

fix: proper catch-all daemon startup errors #2030

Merged
merged 1 commit into from
Mar 14, 2022
Merged

fix: proper catch-all daemon startup errors #2030

merged 1 commit into from
Mar 14, 2022

Conversation

lidel
Copy link
Member

@lidel lidel commented Mar 14, 2022 •
edited
Loading

This PR removes false-negatives when "error" is printed by ipfs daemon but it does not kill the process.

This updated logic will set isErrored only when ipfs daemon is truly unable to continue:

  • checks if process behind spawned pid is alive, and shows "startup error" window when process is dead
  • removes false-negatives caused by migration code or other "soft" errors printed to the console
  • turns the isErrored state into generic "daemon startup has failed" one

Closes #2023
Closes #2018
Closes #2027
Closes #2017
Closes #2028

  • prob many more

TODO

  • remove string matching, switch to pid inspection
  • test how IPFS fetch behaves when gateway at ipfs.io is broken (but dist.ipfs.io DNSLink still works)
    • fetching from IPFS works as expected
  • how does this impact $IPFS_PATH/api pointing at remote node
    • it works fine, subprocess is null, skipping pid checks

@lidel lidel marked this pull request as ready for review March 14, 2022 21:29
@lidel
fix: proper catch-all daemon startup errors
daeaffc 
this should engage isErrored only when ipfs daemon is truly errored:
- checks if pid is alive, and shows "startup error" window when process
  is dead
- removes false-negatives caused by migration code or other "soft"
  errors printed to the console
@lidel lidel force-pushed the fix/catch-all-daemon-startup branch from 4333dea to daeaffc Compare March 14, 2022 21:33
@lidel lidel requested a review from hacdias March 14, 2022 23:04
@lidel lidel mentioned this pull request Mar 14, 2022
Closed
@lidel lidel merged commit f99aa5f into main Mar 14, 2022
@lidel lidel deleted the fix/catch-all-daemon-startup branch March 14, 2022 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hacdias hacdias Awaiting requested review from hacdias

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@lidel