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

Bump dexie dep for bugfix, improve supabase example #5469

Merged
merged 5 commits into from Jan 10, 2024

Conversation

marbemac
Copy link
Contributor

@marbemac marbemac commented Jan 5, 2024

close #5466

This PR contains:

  • A BUGFIX: upgrade dexie to resolve flakey Internal error opening backing store for indexedDB.open error in tests
  • SOMETHING ELSE: upgrade the supabase example + hopefully reduce time it takes for the related CI job to complete

Open questions:

  • The bump from alpha dexie to beta.2. Hopefully it is safe but honestly I did not go through their entire changelog and I don't know enough about rxjs internals to have a gut feel re what changes might be risky.
  • Do we want to bump to the latest while we're at it (beta.6) rather than the specific version that includes the fix (beta.2)?

and resolve flakey supabase ci tests

Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com>
Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com>
Copy link
Contributor Author

@marbemac marbemac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In summary:

  • update dexie to gracefully handle a rare chromium indexdb bug that the supabase e2e test was occasionally triggering (and thus failing)
  • while i was at it I updated supabase and modernized the approach to starting it etc. fingers crossed that this has the side effect of reducing CI times

package.json Outdated
@@ -442,7 +442,7 @@
"broadcast-channel": "7.0.0",
"crypto-js": "4.2.0",
"custom-idle-queue": "3.0.1",
"dexie": "4.0.0-alpha.4",
"dexie": "4.0.1-beta.2",
Copy link
Contributor Author

@marbemac marbemac Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the crux of the issue in the flakey test - see the thread in dexie/Dexie.js#543 (comment) for more information.

Before, during the supabase e2e test (only happens sometimes, hence the flake):

Screenshot 2024-01-04 at 2 17 36 PM

After, during the supabase e2e test (still only happens sometimes, but now handled gracefully by dexie):

Screenshot 2024-01-04 at 3 52 37 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note - does the premium indexdb plugin also handle this chromium bug? If not, might want to check out dexie's solution :).

Comment on lines +7 to +17
window.addEventListener('unhandledrejection', (e) => {
console.error(
`got unhandled error`,
e.reason.message,
JSON.stringify({
// @ts-expect-error ignore
frame: e.currentTarget?.ifrm?.id,
}),
e
);
});
Copy link
Contributor Author

@marbemac marbemac Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was difficult to get to the bottom of the issue, because the page was throwing unhandled exceptions and test cafe only captures logs, not exceptions. So the test was effectively silently failing, and test cafe was just going into a loop trying to get it to pass until the test finally timed out.

Now if there are any unhandled exceptions on the page, it's logged as an error and the test cafe tests will immediately stop and print out the relevant info - should make future problems much easier to debug.

Given how test cafe seems to work - might want this same sort of thing on the rest of the e2e tests, where relevant...

@marbemac marbemac marked this pull request as ready for review January 5, 2024 18:13
@marbemac marbemac changed the title fix(deps): Bump dexie dep for bugfix, improve supabase example Jan 5, 2024
Comment on lines -9 to -15
until [ "$n" -ge 20 ]
do
pg_restore -h localhost -d postgres -U postgres dump.sql && break
n=$((n+1))
echo "failed to import, will try again in 5 seconds"
sleep 5
done
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah almost forgot - this loop was always running the max 20 count - it's why the import db step in CI takes ~2m every time.

@pubkey
Copy link
Owner

pubkey commented Jan 9, 2024

@marbemac
Wow, thank you for the investigation and fixes.

Atm the CI fails with npm ERR! Missing script: "start:supabase" please check.

Do we want to bump to the latest while we're at it (beta.6) rather than the specific version that includes the fix (beta.2)?

Yes I think we should directly use beta.6.

The bump from alpha dexie to beta.2. Hopefully it is safe but honestly I did not go through their entire changelog and I don't know enough about rxjs internals to have a gut feel re what changes might be risky.

I will check the basic behavior to ensure current alpha state is compatible with beta.6 state.

does the premium indexdb plugin also handle this chromium bug?

Unlikely, the indexeddb premium has a different way of opening storages compared to dexie so it might not be relevant, but I will check.

Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com>
Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com>
@marbemac
Copy link
Contributor Author

marbemac commented Jan 9, 2024

Atm the CI fails with npm ERR! Missing script: "start:supabase" please check.

Yes I think we should directly use beta.6.

Hopefully all set!

@pubkey pubkey merged commit 453d9f6 into pubkey:master Jan 10, 2024
21 checks passed
@pubkey
Copy link
Owner

pubkey commented Jan 10, 2024

@marbemac Thank you for your work.
I did some checks using the alpha.2 with the beta.6 code and all worked. So I merged this.
For your free access to the premium plugins, can you send me your address on discord so that I can send you the license agreement?

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

Successfully merging this pull request may close these issues.

Planning to tackle the "Fix the flaky tests for the "example-supabase" CI" task
2 participants