dwyl / mvp

📲 simplest version of the @dwyl app
https://mvp.fly.dev
GNU General Public License v2.0
87 stars 2 forks source link

Bug/Chore: Review App Script Failing #373

Closed nelsonic closed 1 year ago

nelsonic commented 1 year ago

Sadly, on the most recent PR: https://github.com/dwyl/mvp/pull/354 the Review Apps Script is failing consistently:

https://github.com/dwyl/mvp/actions/runs/5023363276/jobs/9015775607#step:5:1493

image

Appears to be a DB issue:

ERROR! Config provider Config.Reader failed with:
  ** (RuntimeError) environment variable DATABASE_URL is missing.
  For example: ***HOST/DATABASE
      (stdlib 3.17.2.1) erl_eval.erl:685: :erl_eval.do_apply/6
      (stdlib 3.17.2.1) erl_eval.erl:446: :erl_eval.expr/5
      (stdlib 3.17.2.1) erl_eval.erl:123: :erl_eval.exprs/5
      (elixir 1.14.1) src/elixir.erl:288: :elixir.eval_forms/3
      (elixir 1.14.1) lib/module/parallel_checker.ex:107: Module.ParallelChecker.verify/1
      (elixir 1.14.1) lib/code.ex:422: Code.validated_eval_string/3
      (elixir 1.14.1) lib/config.ex:288: Config.__eval__!/3
      (elixir 1.14.1) lib/config/reader.ex:92: Config.Reader.read!/2
  {"init terminating in do_boot",{#{'__exception__'=>true,'__struct__'=>'Elixir.RuntimeError',message=><<101,110,118,105,114,111,110,109,101,110,116,32,118,97,114,105,97,98,108,101,32,68,65,84,65,66,65,83,69,95,85,82,76,32,105,115,32,109,105,115,115,105,110,103,46,10,70,111,114,32,101,120,97,109,112,108,101,58,32,101,99,116,111,58,47,47,85,83,69,82,58,80,65,83,83,64,72,79,83,84,47,68,65,84,65,66,65,83,69,10>>},[{erl_eval,do_apply,6,[{file,"erl_eval.erl"},{line,685},{error_info,#{module=>'Elixir.Exception'}}]},{erl_eval,expr,5,[{file,"erl_eval.erl"},{line,446}]},{erl_eval,exprs,5,[{file,"erl_eval.erl"},{line,123}]},{elixir,eval_forms,3,[{file,"src/elixir.erl"},{line,288}]},{'Elixir.Module.ParallelChecker',verify,1,[{file,"lib/module/parallel_checker.ex"},{line,107}]},{'Elixir.Code',validated_eval_string,3,[{file,"lib/code.ex"},{line,422}]},{'Elixir.Config','__eval__!',3,[{file,"lib/config.ex"},{line,288}]},{'Elixir.Config.Reader','read!',2,[{file,"lib/config/reader.ex"},{line,92}]}]}}
  init terminating in do_boot ({,[{erl_eval,do_apply,6,[{_},{_},{_}]},{erl_eval,expr,5,[{_},{_}]},{erl_eval,exprs,5,[{_},{_}]},{elixir,eval_forms,3,[{_},{_}]},{Elixir.Module.ParallelChecker,verify,1,[{_},{_}]},{Elixir.Code,validated_eval_string,3,[{_},{_}]},{Elixir.Config,__eval__!,3,[{_},{_}]},{Elixir.Config.Reader,read!,2,[{_},{_}]}]})
  Crash dump is being written to: erl_crash.dump...done
  Starting clean up.
  [    2.143961] reboot: Restarting system
  machine restart policy set to 'no', not restarting
Error: release command failed - aborting deployment. error release_command machine 4d89169da07d78 exited with non-zero status of 1

Given that the WebSockets of Review Apps haven't been working for a while ... and a LiveView App is all about WebSockets. I vote that we either:

  1. Dedicate the time to fixing this and documenting it comprehensively.

OR

  1. Temporarily Disable the Review App Job so that CI stops failing (a false negative for all our Commits/PRs) and revisit this once we have a bit more time.

@SimonLab / @LuchoTurtle what is your preference? 💭 (Thanks! 🙏)

SimonLab commented 1 year ago

Some flyctl commands might have been updated. To speed up the PRs checks and deployment I think it's easier to disable the PR review script for now

nelsonic commented 1 year ago

@SimonLab thanks for checking. 👌 If you have T5m, please create the PR to temporarily disable the Review Script. 🧑‍💻 We definitely want it to be fixed and working in the near future though. 🤞

LuchoTurtle commented 1 year ago

I've found this link of a person having the same issue as us: https://elixirforum.com/t/clean-deploy-to-fly-io-fails-with-database-url-missing-error/52881.

It's obviously a database problem that shouldn't happen because flyctl should set this env when packaging the app.

nelsonic commented 1 year ago

@LuchoTurtle do you think it's fixable in T1h? 💭

LuchoTurtle commented 1 year ago

@nelsonic Don't disable the Review Script. It is used to create the API requests in the API Definition test stage.

LuchoTurtle commented 1 year ago

@nelsonic I can try but my experience with Fly.io is a bit limited. I've been researching and there isn't much to go by. From the link I've sent,

I had issues where the Postgres DB wasn’t being stood up before the deployment finished.

LuchoTurtle commented 1 year ago

Maybe this might be the problem that is halting the database and not allowing flyctl to attach the database to the application (which is the step that ultimately sets the DATABASE_URL, according to https://fly.io/docs/elixir/getting-started/legacy/#attach-our-app-to-the-database.

image

This pertains the mvp-pr-234 from the #234 PR, which is already closed. Should I delete this?

image

This "authentication" error has been looping for the past days, apparently.

LuchoTurtle commented 1 year ago

I think it worked, given https://github.com/dwyl/mvp/pull/375 passed. @nelsonic , please check the PR to merge it to close off this issue 👌

nelsonic commented 1 year ago

Please confirm it’s working to DELETE review apps that are no longer needed.

LuchoTurtle commented 1 year ago
image

The #375 PR appears to have been deleted, so I assume it does.

nelsonic commented 1 year ago

Cool. Thanks. 🙏