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]: 404 Not Found inside exploration player/lesson player for resource used in midi-js #19733

Open
jnvtnguyen opened this issue Feb 12, 2024 · 7 comments
Labels
bug Label to indicate an issue is a regression Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@jnvtnguyen
Copy link
Contributor

Describe the bug

When going to the exploration player or lesson player, I get a 404 error for https://www.oppia.org/dist/oppia-angular/midi/examples/soundfont/acoustic_grand_piano-ogg.js.

URL of the page where the issue is observed.

Exploration/Lesson Player

Steps To Reproduce

  1. Go to any exploration

Expected Behavior

I expect there to be no 404 error.

Screenshots/Videos

Screenshot 2024-02-12 111637

What device are you using?

Desktop

Operating System

Windows

What browsers are you seeing the problem on?

No response

Browser version

No response

Additional context

No response

Tips for developers

Before addressing the bug, please identify which PR caused the issue (you can follow the steps here). If you identify the PR, comment on the issue with a link to it. If not, mention the commit hash of the oldest commit you saw the bug on (and the month and year it was made in).

Also, if this is your first issue, please make sure to follow https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue and https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#setting-things-up before claiming it. Thanks!

@jnvtnguyen jnvtnguyen added triage needed bug Label to indicate an issue is a regression labels Feb 12, 2024
@jnvtnguyen jnvtnguyen changed the title [BUG]: 404 Not Found inside exploration player/lesson player for midi-js [BUG]: 404 Not Found inside exploration player/lesson player for resource used in midi-js Feb 13, 2024
@seanlip seanlip added Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Feb 25, 2024
@sezavala
Copy link

I’m working on this!

@sezavala
Copy link

Question about the link, where did you get the link from? Is the link somewhere in the website right now?

@cjt101
Copy link

cjt101 commented Jul 2, 2024

While working on this issue, I found that this bug was reproducible on both the deployed Oppia server and the Oppia test server. However, this bug does not appear on the local development build of Oppia. I think that this is due to differences in how data is stored in production mode and dev mode, as outlined in this section of the codebase overview.

Additionally, I believe that resolving this issue will also resolve #19928 since the bug there is caused by not being able to find the MIDI file. Likewise, the referenced issue is also not reproducible on the local development build. This has been tested through creating a dummy exploration that contains the music note interaction, and successfully being able to play back music notes.

I would like to try running a local production build of Oppia in order to figure out how this bug is occurring. However, I have come across problems while trying to run the production build. I have opened discussion post #20604 that outlines the problems I face. Post #20607 by @sezavala is also relevant, since starting up the production build of Oppia is also unsuccessful on a different OS (though with different errors). If we are able to successfully set up the production build, we believe that we can make progress in solving this issue.

@cjt101
Copy link

cjt101 commented Jul 9, 2024

After rebuilding my local copy of Oppia using the Linux/Python instructions (instead of Docker), I was able to get both the development and production builds running. The bug still cannot be reproduced on either build, however. So, I think that the source of this bug can only be seen on the deployed builds of Oppia. Despite this, I think I have identified a fix for this bug.

#18599 was the latest change to the MIDI.js files, and it was suggested that moving the midi folder to assets would be another way to migrate MIDI.js functionality. Ultimately the PR did not do this, and it's not specifically stated why. Since MIDI.js needs a URL that routes to the soundfont files, I believe that moving the midi folder to the assets folder and updating the URL string in midi-js.import.ts is a way to solve the current bug regardless of the specific build mode. I have tested this out on my local copy, and indeed the resources are loaded successfully with the new URL.

@jnvtnguyen @seanlip PTAL, thank you! If this looks good, I will create a PR making these changes.

@seanlip
Copy link
Member

seanlip commented Jul 26, 2024

Hi @cjt101, thanks for your note on this issue, and I'm sorry about the late reply. I just came back after some time away and am behind on responding to PRs!

Thank you also for the detailed investigation, I think this is generally along the right lines in terms of identifying the source of the bug. The only concern I have is that we should keep third-party code separate from Oppia-sourced code -- so I'm not entirely sure about moving the midi/ folder to assets/ if the midi/ folder is in third-party.

A question: when you made a production build, were you able to see whether the relevant files were contained in either of the following places?

  • dist/oppia-angular/midi/examples
  • dist/oppia-angular-prod/midi/examples

If so, I am wondering if those files were not uploaded to the live server for some reason -- I looked at .gcloudignore but they don't seem to be being ignored there, from what I can tell. But just wanted to check this with you (and I'll also contact the release team for confirmation too). I'm wondering if maybe they are in one path but not the other, or something like that (e.g. do we need to change a URL somewhere from oppia-angular to oppia-angular-prod when we're looking at the prod server).

Thanks!

@seanlip
Copy link
Member

seanlip commented Jul 26, 2024

Update: Actually, I figured out what the problem is! You can actually check whether a file is uploaded just by going to the relevant URL directly. It looks like

https://www.oppia.org/dist/oppia-angular/midi/examples/soundfont/acoustic_grand_piano-ogg.js

does not exist, but

https://www.oppia.org/dist/oppia-angular-prod/midi/examples/soundfont/acoustic_grand_piano-ogg.js

does. So the main thing that's needed is to change the URL to point to oppia-angular-prod instead in the GAE production environment.

@cjt101 does this give enough info to help address the issue properly (I think the way above is conceptually better than copying files to assets/)? Please let me know if you have any questions about my comments here, thanks!

@cjt101
Copy link

cjt101 commented Jul 26, 2024

Hi @seanlip, thank you so much for your response! I have come up with two different ways of solving this issue with the information you provided. The first one would be to change the hard-coded value of soundfontUrl such that it points to the production build folder, oppia-angular-prod. We make this change in oppia/core/templates/third-party-imports/midi-js.import.ts:

image

However, the 404 error we see with the current production build would now be seen in the development build instead. This is because the oppia-angular-prod folder is only built once the user runs the build script with production mode enabled at least once. So, although this change would fix the 404 error on the deployed website it does not completely fix the issue for all build types.

The second way to solve this issue would is to obtain the current build mode from AppConstants and change the value of soundfontUrl depending on what build mode Oppia was run in:

image

This solution does work for both the development and production builds, since the URL used would correspond to whatever build is currently deployed according to AppConstants.

I have tested both solutions on my local copy of Oppia, and they both work (except for the case I describe with the first solution). Which implementation do you think is more appropriate in solving this issue? I think that the second solution would be better since it fixes the issue across all builds, though there may be a more elegant way of implementing it. Please let me know if you have any suggestions or questions about either solution. Thank you!

Edit: For clarification, neither solution outlined here moves the midi folder. So, files don't need to be copied to assets for either solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Label to indicate an issue is a regression Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Projects
Status: Todo
Development

No branches or pull requests

4 participants