neurobagel / old-query-tool

User interface for searching across Neurobagel graph
https://query.neurobagel.org/
MIT License
2 stars 0 forks source link

[BUG] Safeguard against `undefined` modalities #305

Closed rmanaem closed 11 months ago

rmanaem commented 11 months ago

Is there an existing issue for this?

Expected Behavior

The app should behave the same in both environments.

Current Behavior

When running the app in dev mode on the main branch the app seems to break and respond with a rather ambiguous error once the Submit Query button is clicked. However, the app runs just fine when running in production and was built by running npm run generate and npm run start

Error message

Screenshot from 2023-11-28 17-34-01

image

Environment

No response

How to reproduce

npm run dev on the main branch, then run an empty query vs npm run generate and npm run start, then run an empty query

Anything else?

No response

surchs commented 11 months ago

good catch @rmanaem. It would be good to understand the difference. But more importantly we should remove the local non-static build scripts and make sure that we develop, test, and then run all in the same way that we deploy. So in other words: the fix for this is not so much to make npm run dev succeed again, but rather to replace npm run dev in development with npm run generate && npm run start!

surchs commented 11 months ago

I did a little bit of digging, here is what I found:

  1. This is unrelated to development mode or production mode. It only appears as if it was because in production mode the app doesn't crash out. But if you look at the dev console, the errors are the same
  2. The error is coming from the fact that we blindly trust the API to not return null or undefined as an imaging modality and chain off of the modalities with no protection
  3. The API has unfortunately started returning null as an imaging modality: https://github.com/neurobagel/api/issues/232

This breaks all statements of the form modalities[modality] in this section: https://github.com/neurobagel/query-tool/blob/b8df22b97c7680e67c39ea6533baa73133c174fd/components/ResultCard.vue#L71-L74

because modalities[modality] will be undefined.

Although this will be fixed in the API, we will also have to protect here against a modality being null in the future, e.g. by using optional chaining, see "robustness principle".

rmanaem commented 11 months ago

I'll rename this issue and work on a safeguard.