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

SNOW-1532645: getResultsFromQueryId ran into unhandled error #869

Closed
SeyyedKhandon opened this issue Jul 12, 2024 · 12 comments
Closed

SNOW-1532645: getResultsFromQueryId ran into unhandled error #869

SeyyedKhandon opened this issue Jul 12, 2024 · 12 comments
Assignees
Labels
enhancement The issue is a request for improvement or a new feature question Issue is a usage/other question rather than a bug status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team

Comments

@SeyyedKhandon
Copy link

node: v18.17.1
npm: 9.6.7
snowflake-sdk: "^1.11.0"
Macos

The getResultsFromQueryId method does not handle or provide a way to handle errors, let's say we want to run getResultsFromQueryId with complete to avoid dealing with stream etc.

const getQueryResultsById = async <T>(queryId: string) => {
  try {
    const data = await new Promise<T>(async (resolve, reject) => {
      const connection = getConnectionInstance()
      connection.getResultsFromQueryId({
        queryId,

        complete: (error, statement, rows) => {
          if (error || !rows)  return reject(new Error(errorMessage))
          
          resolve(rows as T)
        },
      })
    })
    return { data }
  } catch (e) {
    const error = e as Error
    return { error }
  }
}

And assume we are using it like:

  const { data, error } = await snowflakeManager.getQueryResultsById('01b599c2-0302-d469-0000-6b0983867cf1')
  console.log('getQueryResultsById:', { data, error })

and in this case 01b599c2-0302-d469-0000-6b0983867cf1 is a incorrect value/id, it will ran into unhandled error and wont be caught inside the complete with error

@SeyyedKhandon SeyyedKhandon added the bug Something isn't working label Jul 12, 2024
@github-actions github-actions bot changed the title getResultsFromQueryId ran into unhandled error SNOW-1532645: getResultsFromQueryId ran into unhandled error Jul 12, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka self-assigned this Jul 12, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added the status-triage Issue is under initial triage label Jul 12, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

hi - thank you for pointing this out and the example , we'll take a look

@SeyyedKhandon
Copy link
Author

@sfc-gh-dszmolka also the type of connection.getQueryStatus(queryId) seems to be wrong, because vscode shows it doesnt need await but at the same time it is a promise behind the scene...
Also asyncExec seems to not be a part of StatementOption type.
Do we need to install a https://www.npmjs.com/package/@types/snowflake-sdk? I tried that but it seems it is not compatible too.
Please shed some light here too

@sfc-gh-dszmolka
Copy link
Collaborator

https://www.npmjs.com/package/@types/snowflake-sdk is not from Snowflake, so it is entirely up to you whether you install it or not.
We now have our own types (with some issues as well :( , which issues should be fixed with the next upcoming release. see other open Issues in this repo with the 'awaiting release' label. asyncExec also has been added so maybe if it's an option for you, you could try checking out the latest code from master and experiment with that - or of course wait a bit for the next release)

will update this Issue once i had a chance to actually look into it. thank you for bearing with us !

@sfc-gh-dszmolka
Copy link
Collaborator

so i took an initial look

using a queryId which is a valid uuid (used the same queryId from your example)

connection.execute({
  sqlText: 'select top 10 * from snowflake_sample_data.tpch_sf1.customer',
  asyncExec: true,
  complete: async function (err, stmt, rows)
  {
    let queryId = stmt.getQueryId();
    console.log(`==> queryId: ${queryId}`);
    let wrongQueryId = "01b599c2-0302-d469-0000-6b0983867cf1"; 

    // 2. Get results using the query ID
    connection.getResultsFromQueryId({
      //queryId: queryId,
      queryId: wrongQueryId,
      complete: async function (err, _stmt, rows)
      {
        console.log(rows);
      }
    });
  }
});

ends up polling /monitoring/queries/<wrongQueryId> every 5 seconds for almost 2 minutes where it errors out with:

/test/node_modules/snowflake-sdk/lib/errors.js:529
  const error = new Error();
                ^

Error [ClientError]: Cannot retrieve data. No information returned from server for query 01b599c2-0302-d469-0000-6b0983867cf1
    at createError (/test/node_modules/snowflake-sdk/lib/errors.js:529:17)
    at exports.createClientError (/test/node_modules/snowflake-sdk/lib/errors.js:366:10)
    at Connection.getResultsFromQueryId (/test/node_modules/snowflake-sdk/lib/connection/connection.js:497:24) {
  code: 460002,
  sqlState: undefined,
  data: undefined,
  response: undefined,
  responseBody: undefined,
  cause: undefined,
  isFatal: true,
  externalize: [Function (anonymous)]
}

by 'error handling', what would be the expected behaviour here?

Would this ClientError (and other exceptions) not be catch-able by the wrapping try/catch block you already use, or another specifically for getResultsFromQueryId ? Thank you for providing further details !

@sfc-gh-dszmolka sfc-gh-dszmolka added the status-information_needed Additional information is required from the reporter label Jul 12, 2024
@SeyyedKhandon
Copy link
Author

SeyyedKhandon commented Jul 12, 2024

@sfc-gh-dszmolka How do you catch that error? none of these two will work:

 try {
    connection.getResultsFromQueryId({
      // queryId: queryId,
      queryId: wrongQueryId,
      complete: async function (err, _stmt, rows) {
        if (err) {
          console.log('err1')
        }
      },
    })
  } catch (e) {
    console.log('err2')
  }

@sfc-gh-dszmolka
Copy link
Collaborator

i was wrong, this is indeed not easily manageable with a simple try/catch block. we'll see what can be done here and i'll update this thread once there's any new info available.

@sfc-gh-dszmolka sfc-gh-dszmolka removed the status-information_needed Additional information is required from the reporter label Jul 13, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

so using Snowflake documentation for Executing queries asynchronously and this blog from Ian Segers which I found very helpful in addressing this particular problem; i took a stab at addressing the issue that the Error is thrown from inside an async function (connection. getResultsFromQueryId)

Using below script, I was either able to get the query result for a valid query, or handle the error for an invalid/bad queryId which would throw from inside getResultsFromQueryId.

Disclaimer: below approach is more like intended as an inspiration, definitely not an official, proper solution from 'Snowflake' which would be ready to be implemented in production.

Script:

# cat async_test.js
var snowflake = require('snowflake-sdk');
//snowflake.configure({ logLevel : 'trace'});

async function run() {
    // Create connection with parameters
    var connection = snowflake.createConnection({
        account: process.env.SFACCOUNT,
        username: process.env.SFUSER,
        password: process.env.SFPASS,
        warehouse: process.env.SFWH,
    });

    await new Promise((resolve, reject) => {
        connection.connect(
            function(err, conn) {
                if (err) {
                    console.error('Unable to connect: ' + err.message);
                } else {
                    console.log('Successfully connected to Snowflake.');
                    resolve(conn);
                }
            }
        )
    });

    let queryId;
    let query = 'select top 10 c_custkey from snowflake_sample_data.tpch_sf1.customer;'

    // 1. Execute query with asyncExec set to true
    await new Promise((resolve) => {
        console.log(`==> Launching query ${query}`);
        connection.execute({
            sqlText: query,
            asyncExec: true,
            complete: async function(err, stmt, rows) {
                queryId = stmt.getQueryId(); // Get the query ID
                console.log(`==> queryId: ${queryId}`);
                resolve();
            }
        });
    });
    let invalidQueryId = 'this_is_invalid';
    let validButBadQueryId = '01b599c2-0302-d469-0000-6b0983867cf1';
    let queryIdToGet = validButBadQueryId;

    // 2. Get results using the query ID
    async function tryGetResultsFromQueryId(connection) {
        try {
	    console.log(`==> Trying to get the query status for ${queryIdToGet}`);
            let result = await connection.getResultsFromQueryId({
                queryId: queryIdToGet
            });
            return result
        } catch (e) {
            console.error(`==> Something bad happened: ${e}`);
            return
        }
    }
    await new Promise((resolve, reject) => {
        tryGetResultsFromQueryId(connection)
            .then((statement) => {
                var stream = statement.streamRows();
                stream.on('error', function(err) {
                    reject(err);
                });
                stream.on('data', function(row) {
                    console.log(row);
                });
                stream.on('end', function() {
                    resolve();
                });
            })
            .catch((error) => {
                console.log(`==> Well this is another error: ${error}`)
            });
    });
}

run();

Run with good queryId (letting it come from the actual query executed:

# node async_test.js
{"level":"INFO","message":"[7:09:45.951 AM]: Trying to initialize Easy Logging"}
..
Successfully connected to Snowflake.
==> Launching query select top 10 c_custkey from snowflake_sample_data.tpch_sf1.customer;
==> queryId: <actualQueryId1>
==> Trying to get the query status for <actualQueryId1>
{ C_CUSTKEY: 60001 }
..
{ C_CUSTKEY: 60010 }

Run with invalid or bad queryId:

# node async_test.js
{"level":"INFO","message":"[7:10:42.920 AM]: Trying to initialize Easy Logging"}
..
Successfully connected to Snowflake.
==> Launching query select top 10 c_custkey from snowflake_sample_data.tpch_sf1.customer;
==> queryId: <actualQueryId2>
==> Trying to get the query status for 01b599c2-0302-d469-0000-6b0983867cf1
==> Something bad happened: ClientError: Cannot retrieve data. No information returned from server for query 01b599c2-0302-d469-0000-6b0983867cf1
==> Well this is another error: TypeError: Cannot read properties of undefined (reading 'streamRows')

Would this be something which helps you in implementing error handling ?

@sfc-gh-dszmolka sfc-gh-dszmolka added question Issue is a usage/other question rather than a bug status-triage_done Initial triage done, will be further handled by the driver team and removed bug Something isn't working status-triage Issue is under initial triage labels Jul 15, 2024
@SeyyedKhandon
Copy link
Author

@sfc-gh-dszmolka Thank you, statement stream is actually usful in my case.
ps. I believe this should be still flagged as a bug, because the getResultsFromQueryId with complete callback, does not behave like the other functions such as getQueryStatus or execute in terms of error handling` which may cause servers crash.

@sfc-gh-dszmolka
Copy link
Collaborator

all right; thank you for the feedback, good to hear you found the workaround helpful!
we'll check what we can do here to provide better error handling but at this stage i'm not sure if it will be actually considered a bug or an enhancement request; the developer team will take a look. I'll keep this ticket posted with any new information, if any.

@sfc-gh-dszmolka sfc-gh-dszmolka added the bug Something isn't working label Jul 15, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

we will add an enhancement at #882 for this corner case where the queryid is syntactically valid, but does not exist. Now the driver will fail much faster (instantly) instead retrying it for 20+ times for 2+ minutes, if the server response is indicating no data is available for the particular query.

@sfc-gh-dszmolka sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review enhancement The issue is a request for improvement or a new feature and removed bug Something isn't working labels Aug 13, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

the above improvement (to not retry 20+ times for 2+ minutes, if we know the queryId doesn't exist on the server) has been merged and will be part of the next upcoming release

@sfc-gh-dszmolka sfc-gh-dszmolka added status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-pr_pending_merge A PR is made and is under review labels Aug 29, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

released with snowflake-sdk v1.13.0 in August 2024 release cycle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is a request for improvement or a new feature question Issue is a usage/other question rather than a bug status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

3 participants