dyedgreen / deno-sqlite

Deno SQLite module
https://deno.land/x/sqlite
MIT License
409 stars 36 forks source link

how should i handle no rows in db? #180

Closed ralyodio closed 2 years ago

ralyodio commented 2 years ago

This throws an error if no rows are found:

  static async findByUsername(username: string) {
    const query = db.prepareQuery<any[]>(
      "SELECT username, created_at FROM users WHERE username = :username"
    );

    return await query.oneEntry({ username });
  }

How can I handle this error and just return null?

GET http://localhost:3001/api/1/users/asdf - 7ms
[uncaught application error]: SqliteError - The query did not return any rows.

request: { url: "http://localhost:3001/api/1/users/asdf2", method: "GET", hasBody: false }
response: { status: 404, type: undefined, hasBody: false, writable: true }

    at PreparedQuery.one (https://deno.land/x/sqlite@v3.2.1/src/query.ts:468:15)
    at PreparedQuery.oneEntry (https://deno.land/x/sqlite@v3.2.1/src/query.ts:493:36)
    at Function.findByUsername (file:///home/ettinger/www/grazily.com/grazily-api/src/models/users.ts:32:24)
    at Controller.getUsername (file:///home/ettinger/www/grazily.com/grazily-api/src/controllers/users.ts:95:30)
    at dispatch (https://deno.land/x/oak@v10.4.0/middleware.ts:41:13)
    at https://deno.land/x/oak@v10.4.0/router.ts:1148:20
    at dispatch (https://deno.land/x/oak@v10.4.0/middleware.ts:41:13)
    at composedMiddleware (https://deno.land/x/oak@v10.4.0/middleware.ts:44:12)
    at dispatch (https://deno.land/x/oak@v10.4.0/router.ts:1154:28)
    at dispatch (https://deno.land/x/oak@v10.4.0/middleware.ts:41:13)
dyedgreen commented 2 years ago

You should use all/allEntries, which will return an empty array if no rows are found. Although it might be worth considering changing one to return null if the entry is not found, and only throw if there are more than one entry?

jsejcksn commented 2 years ago

Although it might be worth considering changing one to return null if the entry is not found, and only throw if there are more than one entry?

@dyedgreen I think this would help in avoiding a lot of currently needed try...catch. Do you have plans to implement the change?

dyedgreen commented 2 years ago

We’d probably want to make it first instead, so we can be backwards compatible. But this would be a relatively simple PR. Do you want to have a go at it @jsejcksn? 😊

jsejcksn commented 2 years ago

We’d probably want to make it first instead, so we can be backwards compatible. But this would be a relatively simple PR. Do you want to have a go at it @jsejcksn? 😊

@dyedgreen I'm not familiar with the source, but I'll take a look. Should I create a draft PR to establish a place for communication if I begin and have questions?

dyedgreen commented 2 years ago

Sounds good 😊

dyedgreen commented 2 years ago

Maybe as a side-note: Such a function would simply be a slightly more efficient way to do this:

const [first] = myQuery.all();