Illegal value 'Illegal value' when calling method: [calIRecurrenceInfo::modifyException]
Categories
(Calendar :: Lightning Only, defect)
Tracking
(Not tracked)
People
(Reporter: aleca, Assigned: darktrojan)
References
(Regression)
Details
Attachments
(2 files, 1 obsolete file)
1.35 KB,
patch
|
pmorris
:
review+
aleca
:
feedback-
|
Details | Diff | Splinter Review |
2.27 KB,
patch
|
pmorris
:
review+
|
Details | Diff | Splinter Review |
Multiple console errors show up as soon as Thunderbird gets launched:
console.warn: Lightning: Message: [Exception... "Illegal value'Illegal value' when calling method: [calIRecurrenceInfo::modifyException]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: file:///home/alecaddd/Mozilla/source/obj-x86_64-pc-linux-gnu/dist/bin/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calStorageCalendar.js :: _assureRecurringItemCaches/< :: line 1751" data: no]
Comment 1•5 years ago
|
||
Can you provide steps to reproduce? I'm testing with 71.0a1 (Build ID: 20190917085608). I created multiple repeating events and modified them and I'm not seeing this error.
Reporter | ||
Comment 2•5 years ago
|
||
The errors appear on startup, even if the calendar tab is closed.
I have 2 calendars, 1 local and 1 remote from Fastmail.
I just pulled this morning from m-c and c-c.
Do you need any specific info?
Comment 3•5 years ago
|
||
Hmm, that code at
https://searchfox.org/comm-central/rev/c0fb2575fb46e9036dcf758c4f3962035616602a/calendar/providers/storage/calStorageCalendar.js#1751
was added in bug 501689. We should make sure this gets looked at.
Assignee | ||
Comment 4•5 years ago
|
||
I found a bug that is related, not sure if it's the reported one though.
Reporter | ||
Comment 5•5 years ago
•
|
||
Comment on attachment 9093466 [details] [diff] [review] 1581909-async-recurrenceinfo-1.diff Review of attachment 9093466 [details] [diff] [review]: ----------------------------------------------------------------- Same problems, but with a slightly bigger error message. `console.error: Lightning: recurrenceInfo::addException: item with null recurrenceId! JavaScript error: file:///home/alecaddd/Mozilla/source/obj-x86_64-pc-linux-gnu/dist/bin/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calStorageCalendar.js, line 1754: uncaught exception: 2147942487` `console.warn: Lightning: Message: [Exception... "Illegal value'Illegal value' when calling method: [calIRecurrenceInfo::modifyException]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: file:///home/alecaddd/Mozilla/source/obj-x86_64-pc-linux-gnu/dist/bin/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calStorageCalendar.js :: _assureRecurringItemCaches/< :: line 1754" data: no] `
Assignee | ||
Comment 6•5 years ago
|
||
In hindsight this makes sense, we should empty the cache out before trying to fill it up again. The complaining code is finding something in the cache that shouldn't be there.
I never realised just how many times we call _assureRecurringItemCaches – four at start-up for a calDAV calendar with offline storage. That's got to be bad for performance.
Reporter | ||
Comment 7•5 years ago
|
||
Comment on attachment 9093488 [details] [diff] [review] 1581909-async-clearcache-1.diff Review of attachment 9093488 [details] [diff] [review]: ----------------------------------------------------------------- This does the trick for me!
Comment 9•5 years ago
|
||
Comment on attachment 9093488 [details] [diff] [review] 1581909-async-clearcache-1.diff Review of attachment 9093488 [details] [diff] [review]: ----------------------------------------------------------------- With these two patches applied, the errors no longer appear. I'm not well versed in this part of the code, but the change seems reasonable.
Comment 10•5 years ago
|
||
Comment on attachment 9093466 [details] [diff] [review] 1581909-async-recurrenceinfo-1.diff Review of attachment 9093466 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.
Comment 12•5 years ago
|
||
I think we should get them landed.
Comment 13•5 years ago
|
||
Comment on attachment 9093466 [details] [diff] [review] 1581909-async-recurrenceinfo-1.diff Review of attachment 9093466 [details] [diff] [review]: ----------------------------------------------------------------- ::: calendar/providers/storage/calStorageCalendar.js @@ +1735,5 @@ > + let recInfo = item.recurrenceInfo; > + if (!recInfo) { > + recInfo = cal.createRecurrenceInfo(item); > + item.recurrenceInfo = recInfo; > + } this would be shorter: if (!item.recurrenceInfo) { item.recurrenceInfo = cal.createRecurrenceInfo(item); }
Comment 14•5 years ago
|
||
I'd prefer Geoff to tell use what he wants to do, maybe even change the snippet you've pointed out.
Comment 15•5 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/d52c5f098152
Use existing item recurrence info instead of overwriting it; r=pmorris
https://hg.mozilla.org/comm-central/rev/2fd4b75c6950
Clear in-memory item cache before re-filling it; r=pmorris
Updated•5 years ago
|
Updated•5 years ago
|
Comment 16•5 years ago
|
||
https://hg.mozilla.org/comm-central/rev/2148bb88117e96f83d4c780f2707fc44ed2423f8
Backed out 2 changesets (bug 1581909) for test failures in calendar/test/unit/test_providers.js. a=backout DONTBUILD
Comment 17•5 years ago
|
||
I've seen so much red/orange in the last few hours that I'm confused what's working and what is not. Let's see:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ec7a23f9bcfd63e88fb50a05a8f91c72b7885dc7
Assignee | ||
Comment 18•5 years ago
|
||
IMO it's actually the test that's wrong, but I still should've checked it before pushing. :-/
This is a better fix anyway. It only clears items from mItemCache
that are going to be replaced at the end of the function. Also I realised we've never cleared mRecEventCache
/mRecEventCacheOfflineFlags
/mRecTodoCache
/mRecTodoCacheOfflineFlags
before filling them which could lead to ghost entries. I've never seen a problem with this before but it should be fixed anyway.
Comment 19•5 years ago
|
||
Comment on attachment 9094051 [details] [diff] [review] 1581909-async-clearcache-2.diff Review of attachment 9094051 [details] [diff] [review]: ----------------------------------------------------------------- The changes look fine to me.
Updated•5 years ago
|
Comment 20•5 years ago
|
||
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/cf15df787db9
Use existing item recurrence info instead of overwriting it. r=pmorris
https://hg.mozilla.org/comm-central/rev/a652580e14da
Clear in-memory item cache before re-filling it. r=pmorris
Description
•