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

Uncatachable exception "No toISOString function in exdate[name]" #144

Open
Apollon77 opened this issue Aug 11, 2021 · 21 comments
Open

Uncatachable exception "No toISOString function in exdate[name]" #144

Apollon77 opened this issue Aug 11, 2021 · 21 comments

Comments

@Apollon77
Copy link
Contributor

The error thrown in

throw new TypeError('No toISOString function in exdate[name]', exdate[name]);

is not catchable because anything is executed in a setImmediate before ... There is no way to catch that error

@ElAmarok
Copy link

Any way to fix this?

@ElAmarok
Copy link

Or is there any workaround to keep it working?

@sdetweil
Copy link
Contributor

can u tell if this is related to the misformatted TZ values (MS outlook... like EST5EDT or 'Customized Time Zone'

@sdetweil
Copy link
Contributor

see the workaround I had to add to our applicatrion for rrule returning junk dates
#172 (comment)

@sdetweil
Copy link
Contributor

sdetweil commented Oct 22, 2024

see #348 update to README, relative to the try/catch

@sdetweil
Copy link
Contributor

sdetweil commented Oct 22, 2024

I just fixed the throw messages to include the data .. which was lost in the conversion from console.log() to throw in 12.7

@sdetweil
Copy link
Contributor

@Apollon77 an you confirm v 0.20.0 provides a way to catch this error?

@Apollon77
Copy link
Contributor Author

In my eyes the main topic is that big files are getting curreed and processed "async" using setImmediate to not block the event loop too long

https://github.com/sdetweil/node-ical/blob/afee8ed47376c5f464044bab5299e350a16994a4/ical.js#L752

This means that any error thrown in parseLines or anything that method does which is not also catched and converted into an error callback or such it uncatcheable.

So means when exdateParameter() throws an error then we would need to somehow catch this in

throw new TypeError('No toISOString function in exdate[name]', exdate[name]);
or wherever "END" is called ... I did not verified through all code now ... can you maybe point me to where this is now catched?

@sdetweil
Copy link
Contributor

again, show me an example

i believe the try/ catch as i demonstrated will work

@Apollon77
Copy link
Contributor Author

Ok maybe I miss something ... where you show a try/catch? have the link for me ... so that we talk about the same :)

@sdetweil
Copy link
Contributor

sdetweil commented Oct 30, 2024

see the comment in #172
#172 (comment)

@Apollon77
Copy link
Contributor Author

Apollon77 commented Oct 30, 2024

How this can work?

The async variant uses the "callback" variant of parseIcs which uses parseLines with a limit of 2000 lines per "task". So as long as your file is smaller then this limit all is fine and your approach works. As soon as the file gets >2000 lines then the problem is that the "setImmediate" used in parseLines() is detaching the further processing which leads to anything throws getting uncatcheable.

So the errors would need to be catched in parseLines and converted into callback errors or whatever is wanted here.

or what do I miss?

@sdetweil
Copy link
Contributor

use your own routine, don't call node ical functions direct from setImmediate()

@Apollon77
Copy link
Contributor Author

Sure, i could implement my own "parseIcs" which calls parseLines differently but what's the point then with the library? For all other library users it would still be the same issue.

If you propose that the async variant fixes it we could also adjust the async implementation for be a loop with "await" of parseLine calls instead of the setImmediate thingy because the awaits should fulfill the same purpose to let other async stuff happen in between and not to block the event loop ... then the issue would be solved on library level. That culd be also an solution.

@sdetweil
Copy link
Contributor

sdetweil commented Oct 30, 2024

no

const myfunc = ()=> {
try{   
  node-ical.parselines(....)
}
catch(error){...} 
}

you are not ' implementing' anything extra, just managing the execution environment

then you can catch whatever

setImmediate(myfunc)

on my phone in the car, text not pretty

@Apollon77
Copy link
Contributor Author

In fact the question is what should happen when such an error happens. if this should stop the processing then try catch around

this.parseLines(lines, limit, ctx, stack, i + 1, cb);
and file callback with error should be the solution already

@sdetweil
Copy link
Contributor

well, THAT is a different problem.

i think you should close this issue as it says cant be caught. but it can, given careful error recovery planning for the runtime environment.

then on the other, i think you are welcome to fix/enhance that as a contribution.

ive been making contributions for the last few years, i think they have made the lib better for everyone.

@Apollon77
Copy link
Contributor Author

In fact no i will not close the issue because without a fix the issue exists and should be fixed. If you check also I did several contributions to the library already, so yes all fine. Maybe I can provide a PR somewhen when I find time (so this issue was also to remind myself) beside other higher prioritized topics I am currently working on. And sorry, but no need to tell me to make contributions ... ;-) I manage multiple Open source projects also since years ... maybe simply too many in the meantime.

@sdetweil
Copy link
Contributor

sdetweil commented Oct 31, 2024

wasn't trying to be snooty or anything, don't know you. it was just a serious suggestion on a way forward now that we understand the requirement.

@sdetweil
Copy link
Contributor

weird... I posted message this morning..

you want the error reported thru callback., got it
what do you expect for the parseLines() you started? do you expect it to continue? or just end. (it didn't/can't finish)

@Apollon77
Copy link
Contributor Author

In fact my main thing is that it does not crash in an uncatcheable way. So to "fix" the issue a try/catch would be enough ... but yes then the question is what should happen ... In fact there is "one entry" in the file broken ... should the whole file be aborted because of this? And then it is about "how to report back partial invalid entries" so that the using dev knows.
Currently the process would simply error. so catching and return on callback with error would be sufficient - not nice but "as it was before too"

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

No branches or pull requests

3 participants