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

Server Error #281

Closed
seaneady opened this issue Jan 23, 2023 · 30 comments
Closed

Server Error #281

seaneady opened this issue Jan 23, 2023 · 30 comments
Assignees
Labels
BLOCKED :fire: Core team's HIGHEST priority, blocking critical work bug Suspected or confirmed bug (defect) in the code please-test Please test the feature in Staging Environment and confirm it's working as expected. priority-1 Highest priority issue. This is costing us money every minute that passes. tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written technical A technical issue that requires understanding of the code, infrastructure or dependencies

Comments

@seaneady
Copy link
Collaborator

image
Reporting a bug which I encountered when attempting to sign up to the mobile and web version of the MVP.

@seaneady seaneady added the priority-1 Highest priority issue. This is costing us money every minute that passes. label Jan 23, 2023
@LuchoTurtle
Copy link
Member

🤔

Did this error occur when you logged in? I've tried logging in from an anonymous window and it seems to be working properly.

image

What steps did you do to get to this error?

@LuchoTurtle
Copy link
Member

Hm, okay, when pushing changes to #280, it seems the CI errors when deploying it to Fly.io.

https://github.com/dwyl/mvp/actions/runs/3988547375/jobs/6839852547

This might be related. Let me see if I can figure out what's going on 👍

@LuchoTurtle
Copy link
Member

Uhm. I re-ran that job and it seems to be working now.
https://github.com/dwyl/mvp/actions/runs/3988547375

Perhaps some networking issues on Fly.io's side?
Logs don't show anything out of the ordinary either -> https://fly.io/apps/mvp-pr-280/monitoring

Is your issue still occurring @seaneady ? Everything seems fine on my end 😄

@seaneady
Copy link
Collaborator Author

Screenshot 2023-01-23 at 5 32 12 PM

I tried it in two different browsers and still getting this error.

@seaneady
Copy link
Collaborator Author

also tried it on other computers too and I got the same thing.

@SimonLab
Copy link
Member

I can see the following error on the logs, not sure yet if it's related:
image

@LuchoTurtle
Copy link
Member

Database logs show this. https://fly.io/apps/mvp-db/monitoring

2023-01-23T17:26:07.005 app[12a3726f] lhr [info] keeper | 2023-01-23 17:26:07.002 UTC [32321] DETAIL: Key (person_id)=(209) is not present in table "people".

2023-01-23T17:26:08.021 app[12a3726f] lhr [info] keeper | 2023-01-23 17:26:08.021 UTC [32321] ERROR: insert or update on table "items" violates foreign key constraint "items_person_id_fkey"

This is probably the issue, perhaps Sean's person row was deleted from the database for whatever reason and can't properly login (?).

I don't really know how to access the database and make changes to it, going to leave it to the pros. @nelsonic @SimonLab

@SimonLab
Copy link
Member

From #281 (comment) I think maybe an error has been created on Fly.io side. the auth.dwyl.com is accessible for me but it makes sense that the queries won't work on the mvp if the person_id is not retrieved from the auth app.

@nelsonic
Copy link
Member

@seaneady thanks very much for reporting this. 👌 (😢)
I strongly suspect that this is a browser cookies issue. 🍪
Specifically that something went wrong in the auth process and the JWT is corrupt/outdated. 🙄
Please confirm which web browser you are using so we can help with clearing. 🙏

@seaneady
Copy link
Collaborator Author

No problem. I tried from Chrome and Safari on two separate computers and got the same error. Could it have something to do with the fact that I am trying to log in from South Africa maybe?

@nelsonic
Copy link
Member

It's possible that CloudFlare is blocking the requests. Super lame if they are! 😢
Might need to fire up VPN to see if we can reproduce the error. 💭

@nelsonic nelsonic added bug Suspected or confirmed bug (defect) in the code technical A technical issue that requires understanding of the code, infrastructure or dependencies BLOCKED :fire: Core team's HIGHEST priority, blocking critical work tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written labels Jan 24, 2023
@nelsonic
Copy link
Member

@seaneady when you're back at your desk, please LMK.
We can remote-pair on debugging this.
Continue: https://fly.io/apps/authprod/monitoring

@seaneady
Copy link
Collaborator Author

@LuchoTurtle sorry I'm only getting back to you now. Been a bit hectic on my side today. Would 3pm your time work? I have another call starting now 2pm(your time).

@LuchoTurtle
Copy link
Member

@seaneady sorry, just saw this!
I'm not sure I'll help much "unblocking this", as I don't have much experience with Fly.io as @nelsonic or @SimonLab and I don't want to "break" anything.
But if Nelson pairs with you on this, I can surely tag along!

@nelsonic
Copy link
Member

Let's look at this immediately after standup today (stay on the Zoom call). 👌
We need @seaneady's help to debug because only he is seeing the error. 🙈

@nelsonic
Copy link
Member

We're taking a look at this now on the Zoom call. 🔥

@nelsonic
Copy link
Member

This is going to be a bit of a mission. We need to understand the Database constraint error as shared by @LuchoTurtle above ⏫ #281 (comment)

@SimonLab
Copy link
Member

I think the issue is that we still have a foreign key constraint on the items table for people:
image

So if we remove the items_person_id foreign key it might resolve the issue

@SimonLab
Copy link
Member

SimonLab commented Jan 25, 2023

We can do this directly on postgres with the command:

ALTER TABLE table_name 
   DROP CONSTRAINT foreign_key_name; 

in our case

ALTER TABLE items 
   DROP CONSTRAINT items_person_id_fkey; 

What I'm not sure is why the constraint stayed when we removed it from the migrations. Maybe there is an migration command to do this directly in the Phoenix project.

I think we can create a new migration to use drop, https://hexdocs.pm/ecto_sql/Ecto.Migration.html#drop/2

@SimonLab
Copy link
Member

On my localhost, after running mix ecto.reset I have the following:
image

We can see that I don't have the foreign key defined.
Maybe there was an issue running a migration on fly, I think the easier/quickest way to fix this issue is to run the postgres command directly on fly (using flyctl postgres connect command).

@nelsonic
Copy link
Member

Ok. we have a snapshot created: dwyl/learn-devops#87
I'm going to try and attach the mvp App to the new cluster: debugging-281
and see if we can run this ALTER TABLE command on it. 👌

@nelsonic
Copy link
Member

Applied the ALTER TABLE command on mvp-db and it should no longer have the constraint error ...
@seaneady please test when you can. 🙏

@nelsonic nelsonic added the please-test Please test the feature in Staging Environment and confirm it's working as expected. label Jan 25, 2023
@SimonLab
Copy link
Member

The people table has been removed from the phoenix app without creating a migration. b7d7aed
So on Fly we still have the people table and the foreign keys linked to this table.
On localhost when mix ecto.reset is run the database is ok, because the migration file for people has been deleted.

@nelsonic
Copy link
Member

Yeah, that's my bad. Thought it had been removed cleanly. Thanks for investigating @SimonLab 🔍 🕵️

@LuchoTurtle
Copy link
Member

The people table has been removed from the phoenix app without creating a migration. b7d7aed So on Fly we still have the people table and the foreign keys linked to this table. On localhost when mix ecto.reset is run the database is ok, because the migration file for people has been deleted.

But if the migration file has been deleted from the repo, doesn't Fly.io just run the migrations from scratch? It shouldn't create the people table if no migration file is present in the repo 🤔

@nelsonic
Copy link
Member

nelsonic commented Jan 25, 2023

Nope. Sadly, that's now how it works. It only runs the new migrations. The old ones are already executed.
This is both a benefit and a downside of migrations.

@nelsonic
Copy link
Member

Ok. we need @seaneady to re-test on his side ...
meanwhile I can confirm that everything on the MVP still works for me. 👌

@nelsonic nelsonic removed their assignment Jan 25, 2023
@nelsonic nelsonic moved this to ⏳Awaiting Review in dwyl app kanban Jan 25, 2023
@seaneady
Copy link
Collaborator Author

Tested and can confirm that I can login via Github however when I attempt to login with my email address it still gives me an error message as per screenshot.
Screenshot 2023-01-25 at 2 16 10 PM

@nelsonic
Copy link
Member

@seaneady thanks for confirming that you were able to login with GitHub 🎉
the email issue has been logged as: dwyl/auth#264 📧 🙅 🔥

@nelsonic
Copy link
Member

https://mvp.fly.dev/stats

image

@seaneady looks like you've been able to authenticate and add a couple of items. 👌
Please confirm if you still see the Server Error and if it's no longer an issue, please close. ✅

@nelsonic nelsonic moved this from ⏳Awaiting Review to 👀 In review in dwyl app kanban Jan 28, 2023
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in dwyl app kanban Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLOCKED :fire: Core team's HIGHEST priority, blocking critical work bug Suspected or confirmed bug (defect) in the code please-test Please test the feature in Staging Environment and confirm it's working as expected. priority-1 Highest priority issue. This is costing us money every minute that passes. tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written technical A technical issue that requires understanding of the code, infrastructure or dependencies
Projects
Status: Done
Development

No branches or pull requests

4 participants