search

zakodium-oss / react-science

React components and tools to build scientific applications.
https://react-science.pages.dev
MIT License
3 stars 6 forks source link

Loaders should return log #291

Closed lpatiny closed 8 months ago

lpatiny commented 2 years ago

We have currently an issue tracking errors during data loading. This is the case in this project as well as in NMRium.

For each loader we should be able to return information about what they did.

I would suggest that the loader returns a structure like

{
  data: {} // the result of the parsing, in this case 'Measurements'
  logs: [{
     kind: 'error' | 'warning' | 'info' | 'debug',
     loader: 'NetCDF parser',
     message: 'The CDF is corrupted',
     relativePath: 'data/test/file.cdf',
     error: new Error('ab'),
  }]
}

@stropitek @targos What do you think ?

@jobo322 this is related to your current issue in nmr-load-save.

targos commented 2 years ago

We need something, but what you propose doesn't seem useful because we don't know which file has a problem.

lpatiny commented 2 years ago

I amended my issue to add the new property relativePath. This relativePath could be a link to a folder in case we try to parse a bruker file that contains soo many levels or just the path to a specific file like jcamp or netcdf.

WDYT ?

stropitek commented 2 years ago

Is the goal of this to show a helpful message to the user, or to be able to debug what is happening when files are loaded?

lpatiny commented 2 years ago

In the specific case of convert-biologic you should add in the loader a try / catch so that it does not crash and for each file add an entry in the log, either 'error' if failed or 'info' if success.

ghost commented 2 years ago

The converter already throws internally when the file is not supported.

For the error that the parser throws I'll catch it and log a warning if that is preferred @lpatiny

The other error that I see in most parsers disappears if the crossHairAnnot is commented out.

lpatiny commented 2 years ago

It is expected that the converters (parsers) throws and this should not be changed.

Those errors (converter throws) should be catch during the loading. We could decide that for now the logs of all the loaders are just 'console.log' but we need to think about a feedback to the user in the future and design a component for this purpose.

I'm thinking that adding the property 'error' that contains the error that is catched could also help us to debug in the future what went wrong. You could then display error.stack for advanced users (like us).

lpatiny commented 1 year ago

This issue can now be solved using fifo-logger and showing the errors is related to: