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

Fix problems with local libSQL DB #12089

Merged
merged 12 commits into from
Oct 7, 2024
Merged

Fix problems with local libSQL DB #12089

merged 12 commits into from
Oct 7, 2024

Conversation

Fryuni
Copy link
Member

@Fryuni Fryuni commented Sep 28, 2024

Changes

  • Fixes problems related to using local files as libSQL remote reported on Discord with the sunsetting of Studio
  • Relative paths were being treated as absolute paths and resulting on paths to folder that didn't exist or permission error trying to create a file on the filesystem root
  • Pushing the schema to a new local file DB wasn't working. Pushing to remote server with a local file as embedded replica was.

Testing

  • A new test for each of the problems was added

Docs

The new behavior matches the documentation

@Fryuni Fryuni added the pkg: db label Sep 28, 2024
@Fryuni Fryuni self-assigned this Sep 28, 2024
Copy link

changeset-bot bot commented Sep 28, 2024

🦋 Changeset detected

Latest commit: c5a5e52

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot removed the pkg: db label Sep 28, 2024
@@ -451,7 +451,7 @@ async function getDbCurrentSnapshot(

return JSON.parse(res.snapshot);
} catch (error: any) {
if (error.code === 'SQLITE_UNKNOWN') {
if (error.code === 'SQLITE_UNKNOWN' || (error.code === 'SQLITE_ERROR' && error.rawCode === 1)) {
Copy link
Member

Choose a reason for hiding this comment

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

It's worth leaving a comment here and explaining the check. The comment should explain:

  • why SQLITE_ERROR?
  • why rawCode === 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I went to the source code to understand the error codes. It turns out the rawCode adds nothing here; it is just the numeric value of the enum while code is the name of the value. code: 'SQLITE_ERROR' will always come with rawCode: 1.

Also, SQLITE_ERROR is a generic error code used for multiple things. We'll have to check the message to ensure we only capture the error code we care about.

We'll still need to check for both codes because the library throws two errors when reading a non-existing table:

  • Connected to a remote DB it throws with the code SQLITE_UNKNOWN and message SQLITE_UNKNOWN: SQLite error: no such table: some_table
  • Connected to an in-memory or a local file DB it throws with the code SQLITE_ERROR and message SQLITE_ERROR: no such table: some_table

I'll change the code to properly reflect that.

packages/db/src/runtime/db-client.ts Show resolved Hide resolved
@Fryuni Fryuni merged commit 6e06e6e into main Oct 7, 2024
14 checks passed
@Fryuni Fryuni deleted the fryuni/fix-db-with-local-files branch October 7, 2024 13:17
@astrobot-houston astrobot-houston mentioned this pull request Oct 7, 2024
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.

3 participants