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

Empty get/all results in process termination #1

Closed
yoavain opened this issue Feb 1, 2022 · 6 comments
Closed

Empty get/all results in process termination #1

yoavain opened this issue Feb 1, 2022 · 6 comments

Comments

@yoavain
Copy link

yoavain commented Feb 1, 2022

When get/all returns with 0 rows, result equals "".
The parse in the next line then throws an error, which I think is still part of the spawned process and can't be caught by the caller.

        const json = parse(result);
        res(type === 'get' && isArray(json) ? json.shift() : json);

Maybe should just return an empty result instead of exit the process.

@yoavain
Copy link
Author

yoavain commented Feb 1, 2022

Current workaround: using 'query' instead

@WebReflection
Copy link
Owner

@yoavain can you present a single case where using get or all fails, in term of SQL? Because when you don't want to return any result, query is exactly what anyone should use ... otherwise it makes no sense. get returns always null or undefined, and all returns an empty array ... so when exactly this fails, in terms of expectations, if you SELECT and nothing is found?

@yoavain
Copy link
Author

yoavain commented Feb 2, 2022

There are 2 reason I don't use query:

  1. issue Add to options arguments to be passed to the sqlite process #2 (I have "|" in the content, and there's no way to control the separator)
  2. I prefer getting my response in a JSON structure

My use case is using the DB as cache.
I check if item is in the DB. If it is, I return it, if not, I fetch it from a remote location.
So 'get' was my preferred method...

@WebReflection
Copy link
Owner

I have "|" in the content, and there's no way to control the separator

I don't understand this statement ... this project uses -json and the result is JSON when you select anything

I prefer getting my response in a JSON structure

And that's exactly what you get here. get and all are JSON result parsed as JSON ... unless you are saying you want a reviver?

In few words, query is for raw output, get and all are for SELECT results. There's no way you get non JSON output when no result is found, unless you provide an example that demonstrate what you are trying to do and why you have issues.

Until that, neither issues would be worked on as there's no issue to fix and no example to test to me.

@yoavain
Copy link
Author

yoavain commented Feb 2, 2022

I have tried using get/all. In my case, a row may or may not be in the DB
Here's my code:

try {
    const row = await get`SELECT * from ${schema}.${lyricsTable} WHERE artist=${artist} AND title=${title}`;
    return row && row.lyrics;
}
catch (e) {
    return null;
}

(artist+title is the primary key)

The problem is that if the row does not exist in the DB, and you use JSON.parse() on it, the child process sends a terminates event, which terminate the NodeJS process. (The try/catch doesn't help since the crash is from a child process)

@WebReflection
Copy link
Owner

@yoavain beside the fact I have just published a fix, because indeed no results throws errors, watch out you cannot write this:

get`SELECT * from ${schema}.${lyricsTable} WHERE artist=${artist} AND title=${title}`
//                ^^^^^^^^^^^^^^^^^^^^^^^^

that's a SQL injection hazard ... you need to use the raw helper if you want to compose schema/table!

Anyway, this bug has been fixed, next one please write a test case right away to save time to ourselves ;-)

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

2 participants