yjmantilla / sovabids

A python package for the automatic conversion of EEG datasets to the BIDS standard, with a focus on making the most out of metadata.
https://sovabids.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
9 stars 3 forks source link

"Internal Server Error" in GUI #41

Open civier opened 3 years ago

civier commented 3 years ago

I found out that when there are inconsistencies/missing information in the data, the GUI prints a page with the message below after I submit the rules file (i.e., after pressing on the bottom "submit" button in the page that displays the rule file):

**Internal Server Error**
The server encountered an internal error and was unable to complete your request. Either the server is overloaded or there is an error in the application.

The error message printed in the log of the GUI is:

Extracting parameters from /home/neuro/sov=abids/front/app/_uploads/lemon/sub-010004/RSEEG/sub010004.vhdr...
[2021-08-20 14:20:17,519] ERROR in app: Exception on /edit-rules [POST]
Traceback (most recent call last):
  File "/vnm/miniconda3/lib/python3.8/site-packages/flask/app.py", line 2070, in wsgi_app
    response = self.full_dispatch_request()
  File "/vnm/miniconda3/lib/python3.8/site-packages/flask/app.py", line 1515, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/vnm/miniconda3/lib/python3.8/site-packages/flask/app.py", line 1513, in full_dispatch_request
    rv = self.dispatch_request()
  File "/vnm/miniconda3/lib/python3.8/site-packages/flask/app.py", line 1499, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
  File "front/app/app.py", line 227, in edit_rules
    session['mappings'] = result['Individual']
KeyError: 'Individual'

The cases when it occurred during my testing are the following: 1) one of the data files do not match the regular expression/pattern specified in the path_analysis 2) when one of the .vdhr files is empty (I simply edited the file and deleted all content) 3) when the information of one of the markers in the .vmkr file is missing (I changed "Mk5=Stimulus,S210,25931,1,0" to "Mk5=") I'm sure there are more.

I'm not sure what is happening, but I guess that each time entities cannot be extracted from the filename based on the pattern/regexp, or the mne-python data extraction from the meta-files returns an error, the apply_rues OpenRPC call fails and returns an error. Is that the case?

If yes, the least we should do is catch such an error, and provide meaningful feedback to the user, rather than internal server error message. Does apply_rules return a meaningful error code and message at the moment? If yes, it's worth presenting it to the user, and make sure it includes the path to the problematic file/files. If not, it will be good to implement such a reporting mechanism in apply_rules. I hope that you can already include it in the GSoC version of the project, even if the GUI won't be able to handle it for the time being. That said, if the GUI can simply print this message, that would be great --- the user can then press back in the browser, exclude the problematic file in the File Filtering tab, and then repeat

For the future, the functionality that we discussed is that in such cases, the GUI will provide the user with a partial mapping file for the raw file in question (maybe marking the defective/missing part). The user would be then able to edit the mapping file (or if necessary, complete/fix the raw data file's meta-data if it is corrupt or missing), so as to enable conversion of the odd file after all. I guess it is quite simple to implement when it comes to filenames that do not match the pattern/regexp, but more difficult to implement if it is MNE-python that fails in extracting meta-data. I guess that this functionality could be combined with the functionality of the review feature in apply_rules_to_single_file, but I'm not sure. Will the review option only show the resulting meta-data in BIDS format? or will it also show the source meta-data in its original raw format? If the latter, so in case of error, we can possibly show the user what parts of the source raw meta-data we could not map to BIDS.

yjmantilla commented 3 years ago

Thank you for the detailed report @civier , I will see how to tackle this

yjmantilla commented 3 years ago

@civier

I'm not sure what is happening, but I guess that each time entities cannot be extracted from the filename based on the pattern/regexp, or the mne-python data extraction from the meta-files returns an error, the apply_rues OpenRPC call fails and returns an error. Is that the case?

Yes, the API returns an error but it is not correctly handled by the frontend (it does not check the response but rather just continues with the code and then the gui raises an error when it malfunctions).

**If yes, the least we should do is catch such an error, and provide meaningful feedback to the user, rather than internal server error message. Does apply_rules return a meaningful error code and message at the moment? If yes, it's worth presenting it to the user, and make sure it includes the path to the problematic file/files. If not, it will be good to implement such a reporting mechanism in apply_rules. I hope that you can already include it in the GSoC version of the project, even if the GUI won't be able to handle it for the time being. That said, if the GUI can simply print this message, that would be great ---

Im working on an error page that shows the error raised by the API along with the traceback.

The problematic files is more on the sovabids backend side, Im not sure how to straightforwardly do that.

I will first do the error page and then continue with the other final targets, but if I finish them , then I will get into the message of the problematic file.

the user can then press back in the browser, exclude the problematic file in the File Filtering tab, and then repeat **

I havent tested this press-back idea, im not sure how it performs currently. Let me know if you can test that opnce I push the commit for the error message .

For the future, the functionality that we discussed is that in such cases, the GUI will provide the user with a partial mapping file for the raw file in question (maybe marking the defective/missing part). The user would be then able to edit the mapping file (or if necessary, complete/fix the raw data file's meta-data if it is corrupt or missing), so as to enable conversion of the odd file after all. I guess it is quite simple to implement when it comes to filenames that do not match the pattern/regexp, but more difficult to implement if it is MNE-python that fails in extracting meta-data.

Yes, this is correct. The bad match case is easier than the bad eeg-files case. Im not sure we should try to correct them ourselves.

I guess that this functionality could be combined with the functionality of the review feature in apply_rules_to_single_file, but I'm not sure. Will the review option only show the resulting meta-data in BIDS format? or will it also show the source meta-data in its original raw format? If the latter, so in case of error, we can possibly show the user what parts of the source raw meta-data we could not map to BIDS.

As of now, the 'preview' shows arbitrary stuff I collect here:

https://github.com/yjmantilla/sovabids/blob/6229dbed915f24907a02f803dd22301cd2622d90/sovabids/rules.py#L367-L374

I guess our constraint is that we can show just as much as mne allows us. Anything more (like particular vendors data files) would require us to handle such file ourselves. This means doing a lot of logic that may be only possible for a particular vendor and so on.

So in conclusion, we can show as much as the mne abstraction allows us to. There is though a lot to explore there.

yjmantilla commented 3 years ago

@civier

I updated the current behaviour of the front whenever the RPC API throws an error:

This is in the following commit: https://github.com/yjmantilla/sovabids/commit/9db534ea9f6766b4e381dad0841962dd123dcf2b

Here is the current message whenever a response has an error:

image

For the particular cases you mentioned, I leave here the tracebacks for future reference:

1 Bad Match

{
"jsonrpc": "2.0",
"error": {
"code": 5100,
"message": "Error applying rules."
},
"id": 0
}

Traceback (most recent call last):
File "y:\code\sovabids\sovabids\sovarpc.py", line 57, in apply_rules
mappings = ru.apply_rules(source_path=file_list,bids_path=bids_path,rules=rules,mapping_path=mapping_path)
File "y:\code\sovabids\sovabids\rules.py", line 459, in apply_rules
map,_ = apply_rules_to_single_file(f,rules_copy,bids_path,write=False,preview=False) #TODO There should be a way to control how verbose this is
File "y:\code\sovabids\sovabids\rules.py", line 240, in apply_rules_to_single_file
bids_path = BIDSPath(**entities,root=bids_path)
File "C:\software\anaconda3\envs\sovabids\lib\site-packages\mne_bids\path.py", line 289, in __init__
self.update(subject=subject, session=session, task=task,
File "C:\software\anaconda3\envs\sovabids\lib\site-packages\mne_bids\path.py", line 713, in update
_check_key_val(key, val)
File "C:\software\anaconda3\envs\sovabids\lib\site-packages\mne_bids\utils.py", line 232, in _check_key_val
raise ValueError("Unallowed `-`, `_`, or `/` found in key/value pair"
ValueError: Unallowed `-`, `_`, or `/` found in key/value pair session: SE0/bad_match

2 Empty vhdr

{
"jsonrpc": "2.0",
"error": {
"code": 5100,
"message": "Error applying rules."
},
"id": 0
}

Traceback (most recent call last):
File "y:\code\sovabids\sovabids\rules.py", line 176, in apply_rules_to_single_file
raw = read_raw(f,preload=False)#not write)
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\_read_raw.py", line 95, in read_raw
return readers[ext](fname, preload=preload, verbose=verbose, **kwargs)
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\brainvision\brainvision.py", line 834, in read_raw_brainvision
return RawBrainVision(vhdr_fname=vhdr_fname, eog=eog,
File "<decorator-gen-193>", line 24, in __init__
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\brainvision\brainvision.py", line 72, in __init__
orig_units) = _get_vhdr_info(vhdr_fname, eog, misc, scale)
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\brainvision\brainvision.py", line 435, in _get_vhdr_info
settings, cfg, cinfostr, info = _aux_vhdr_info(vhdr_fname)
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\brainvision\brainvision.py", line 346, in _aux_vhdr_info
_check_bv_version(header, 'header')
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\brainvision\brainvision.py", line 296, in _check_bv_version
raise ValueError(_data_err % (kind, header))
ValueError: MNE-Python currently only supports header versions 1.0 and 2.0, got unparsable ''. Contact MNE-Python developers for support.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "y:\code\sovabids\sovabids\sovarpc.py", line 57, in apply_rules
mappings = ru.apply_rules(source_path=file_list,bids_path=bids_path,rules=rules,mapping_path=mapping_path)
File "y:\code\sovabids\sovabids\rules.py", line 459, in apply_rules
map,_ = apply_rules_to_single_file(f,rules_copy,bids_path,write=False,preview=False) #TODO There should be a way to control how verbose this is
File "y:\code\sovabids\sovabids\rules.py", line 178, in apply_rules_to_single_file
raise IOError(f'MNE couldnt read {f} .')
OSError: MNE couldnt read Y:\code\sovabids\front\app\_uploads\empty_vhdr/TTA0/SSE0/subSU1_AC0_0.vhdr .

3 Missing Marker

{
"jsonrpc": "2.0",
"error": {
"code": 5100,
"message": "Error applying rules."
},
"id": 0
}

Traceback (most recent call last):
File "y:\code\sovabids\sovabids\rules.py", line 176, in apply_rules_to_single_file
raw = read_raw(f,preload=False)#not write)
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\_read_raw.py", line 95, in read_raw
return readers[ext](fname, preload=preload, verbose=verbose, **kwargs)
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\brainvision\brainvision.py", line 834, in read_raw_brainvision
return RawBrainVision(vhdr_fname=vhdr_fname, eog=eog,
File "<decorator-gen-193>", line 24, in __init__
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\brainvision\brainvision.py", line 107, in __init__
annots = read_annotations(mrk_fname, info['sfreq'])
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\annotations.py", line 801, in read_annotations
annotations = _read_annotations_brainvision(fname, sfreq=sfreq)
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\brainvision\brainvision.py", line 264, in _read_annotations_brainvision
onset, duration, description, date_str = _read_vmrk(fname)
File "C:\Users\user\AppData\Roaming\Python\Python38\site-packages\mne\io\brainvision\brainvision.py", line 222, in _read_vmrk
mtype, mdesc, this_onset, this_duration = info_data[:4]
ValueError: not enough values to unpack (expected 4, got 1)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "y:\code\sovabids\sovabids\sovarpc.py", line 57, in apply_rules
mappings = ru.apply_rules(source_path=file_list,bids_path=bids_path,rules=rules,mapping_path=mapping_path)
File "y:\code\sovabids\sovabids\rules.py", line 459, in apply_rules
map,_ = apply_rules_to_single_file(f,rules_copy,bids_path,write=False,preview=False) #TODO There should be a way to control how verbose this is
File "y:\code\sovabids\sovabids\rules.py", line 178, in apply_rules_to_single_file
raise IOError(f'MNE couldnt read {f} .')
OSError: MNE couldnt read Y:\code\sovabids\front\app\_uploads\missing_marker/TTA0/SSE1/subSU0_AC1_1.vhdr .
civier commented 3 years ago

Regarding: "I havent tested this press-back idea, im not sure how it performs currently. Let me know if you can test that opnce I push the commit for the error message ."

I've already tested the press-back and it works fine. So when the error message will also include the problematic file, the user would at least be able to exclude it, and then convert the rest of the files. That is a big improvement compared with not being able to convert anything in case of error, so worth implementing :-)

yjmantilla commented 3 years ago

Ahh thats awesome. So yeap, this means that somehow returning the problematic file would be really ideal.