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

[ROMM-1348] Log when a user downloads a game #1350

Merged
merged 3 commits into from
Dec 9, 2024
Merged

[ROMM-1348] Log when a user downloads a game #1350

merged 3 commits into from
Dec 9, 2024

Conversation

gantoine
Copy link
Member

@gantoine gantoine commented Dec 9, 2024

No description provided.

@gantoine gantoine requested review from zurdi15 and adamantike and removed request for zurdi15 December 9, 2024 02:41
Copy link

github-actions bot commented Dec 9, 2024

Test Results

88 tests   88 ✅  26s ⏱️
 1 suites   0 💤
 1 files     0 ❌

Results for commit 60283c3.

♻️ This comment has been updated with latest results.

@adamantike
Copy link
Collaborator

I'm getting a 500 error when the request has no user. To reproduce:

  1. Set DISABLE_DOWNLOAD_ENDPOINT_AUTH=true.
  2. Copy the download link for a game.
  3. Open an incognito tab and access the link from there.
  File "/backend/endpoints/rom.py", line 258, in get_rom_content
    log.info(f"User {current_user.username} is downloading {rom.file_name}")
                     ^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'username'

Copy link
Member

@zurdi15 zurdi15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not better to just have one log before all the ifs and returns instead of inside each one?

@gantoine gantoine requested a review from zurdi15 December 9, 2024 14:54
@gantoine gantoine merged commit 530e7dd into master Dec 9, 2024
8 checks passed
@gantoine gantoine deleted the romm-1348 branch December 9, 2024 23:59
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