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

[Bug] Internal Server Error from data-infographics API #2736

Closed
atalyaalon opened this issue Dec 18, 2024 · 10 comments · Fixed by #2746
Closed

[Bug] Internal Server Error from data-infographics API #2736

atalyaalon opened this issue Dec 18, 2024 · 10 comments · Fixed by #2746
Assignees
Labels
Milestone

Comments

@atalyaalon
Copy link
Collaborator

atalyaalon commented Dec 18, 2024

Describe the bug
when querying this news flash - news flash id=231352

When looking at newsflash API street1_hebrew has \t in it.
I think that's that's one problem we need to fix, perhaps with strip func before adding yishuvs/streets to DB (but not all of it, I think there is another problem):
In addition, when using the search API for this street, we get the same Internal Server Error. Does this mean that this street has no data? https://anyway-infographics-staging.web.app/cityAndStreet/%D7%A2%D7%A8%D7%A2%D7%A8%D7%94/%D7%90%D7%9C%20%D7%93%D7%94%D7%A8%D7%90%D7%AA

To Reproduce
API: https://www.anyway.co.il/api/infographics-data?lang=he&news_flash_id=231352&years_ago=5

Expected behavior
Return widgets if there are any for the newsflash location, and no widgets (empty dict as json) if there aren't any for the newsflash location

Screenshots
Screenshot 2024-12-18 at 12 58 17

Environment
System: Mac

Additional context
Add any other context about the problem here.

@atalyaalon atalyaalon added the bug label Dec 18, 2024
@atalyaalon atalyaalon added this to the v0.18.0 milestone Dec 18, 2024
@atalyaalon
Copy link
Collaborator Author

@ziv17 can you take a look?

@atalyaalon atalyaalon changed the title [Bug] Internal Server Error from API [Bug] Internal Server Error from data-infographics API Dec 18, 2024
@ziv17
Copy link
Collaborator

ziv17 commented Dec 18, 2024

Hi @atalyaalon ,
What should be returned if a news_flash_id parameter is provided, and a newsflash with the provided ID is not found?

@ziv17
Copy link
Collaborator

ziv17 commented Dec 18, 2024

Hi @atalyaalon ,
Look at anyway/flask_app.py:1290. There is code there that specifically returns error if the returned widget list is empty. Should it be removed?

@atalyaalon
Copy link
Collaborator Author

atalyaalon commented Dec 18, 2024

Hi @atalyaalon , What should be returned if a news_flash_id parameter is provided, and a newsflash with the provided ID is not found?

I don't think this is the case - since the current news flash id 231352 exists in production

However, if it's not found I guess we should return some kind of error, perhaps 404 code?

@atalyaalon
Copy link
Collaborator Author

Hi @atalyaalon , Look at anyway/flask_app.py:1290. There is code there that specifically returns error if the returned widget list is empty. Should it be removed?

I think we need differ between 2 cases:

  1. Location exists but no accidents are in our DB
  2. Location does not exist

For the first case, which, I believe is our current case, I suggest that for now, sending an empty list is fine, and we might want to send in the future a special widget indicating that no accidents in DB for this location.

For the second case - I think that sending this error is OK

Is it easy to differ these two cases?

@ziv17
Copy link
Collaborator

ziv17 commented Dec 19, 2024

Hi @atalyaalon ,

I think we need differ between 2 cases:

1. Location exists but no accidents are in our DB

2. Location does not exist

Should we differ between location does not exist and given newsflash id does not exist? If yes, in what way? because currently our error message is "The requested URL was not found".

@ziv17
Copy link
Collaborator

ziv17 commented Dec 19, 2024

Hi @atalyaalon ,

I think we need differ between 2 cases:

1. Location exists but no accidents are in our DB

2. Location does not exist
  1. Should we differ between location does not exist and given newsflash id does not exist?
  2. Should we issue some meaningful error message?
  3. Should we respond differently if we had an internal error (e.g. status 500), or request parameters error / location not found?

@atalyaalon
Copy link
Collaborator Author

atalyaalon commented Dec 19, 2024

Hi @atalyaalon ,

I think we need differ between 2 cases:

1. Location exists but no accidents are in our DB

2. Location does not exist

Should we differ between location does not exist and given newsflash id does not exist?

I guess it would be a best practice, either to differ or to have a debug log for that
If it's not too complicated of course. Otherwise we don't have to differ

@ziv17
Copy link
Collaborator

ziv17 commented Dec 19, 2024

Hi @atalyaalon ,
This newsflash location has issues:

  1. It does not have numeric values for the location fields
  2. The street name has a tab (\t) appended to it, so it is not recognized.

@ziv17
Copy link
Collaborator

ziv17 commented Dec 19, 2024

To summarize:

  1. If newsflash not found, or address not found, we return 404, URL not found.
  2. If we find the address, but no accidents, we return empty data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants