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

queryParameters #499

Closed
Fr4nSG opened this issue Oct 19, 2022 · 7 comments
Closed

queryParameters #499

Fr4nSG opened this issue Oct 19, 2022 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@Fr4nSG
Copy link

Fr4nSG commented Oct 19, 2022

Previous version of C4R: 1.3.0
Actual version of C4R: 1.5.0-alpha.1

executeSQL from @carto/react-api

Error:

{
  "error": "Invalid queryParameters, it must be a list of parameters",
  "status": 400,
  "message": "Invalid queryParameters, it must be a list of parameters"
}

Payload:

{
  "client": "carto-react",
  "q": "SELECT ....",
  "queryParameters": "[]",
  "access_token": "eyJhbGciOiJSUzI1NiIsInR..."
}

Seems like in the new version, the queryParameters is required to specify any value to avoid 400 error like this:

 const data = await executeSQL<GetCategoriesRes | Record<string, number>[]>({
    // @ts-ignore
    credentials,
    query,
    connection,
    queryParameters: null, // TODO: avoid error
    opts: { abortController },
  });

I just check inside and i found the posible bug is on the SQL.js function createRequest. (line.58)

function createRequest({ credentials, connection, query, opts = {}, **queryParameters = []** }) {
  const { abortController, ...otherOptions } = opts;

  const { apiVersion = API_VERSIONS.V2 } = credentials;

  const rawParams = {
    client: CLIENT,
    q: query?.trim(),
    ...otherOptions,
    queryParameters: queryParameters ? JSON.stringify(queryParameters) : undefined
  };

If i downgrade the versions i don't have this issue.

@Fr4nSG Fr4nSG added the bug Something isn't working label Oct 19, 2022
@Josmorsot
Copy link
Contributor

Related with #524. Fixed in #525

@Josmorsot
Copy link
Contributor

v1.4.4 released

@jantolg
Copy link
Contributor

jantolg commented Nov 11, 2022

@Josmorsot I think is not resolved.

The problem ocurrs when is not include any queryParameters, function will include by default [], the evaluation will be true and this one will include queryParameters: '[]', in case queryParemeters will be false, null or undefined, evaluation will return undefined and this will be included in queryParamseters as queryParameters: undefined . Both cases will return error from API

Source: https://github.com/CartoDB/carto-react/blob/master/packages/react-api/src/api/SQL.js:

function createRequest({ credentials, connection, query, opts = {}, queryParameters = [] }) {
  const { abortController, ...otherOptions } = opts;

  const { apiVersion = API_VERSIONS.V2 } = credentials;

  const rawParams = {
    client: CLIENT,
    q: query?.trim(),
    ...otherOptions,
    queryParameters: queryParameters ? JSON.stringify(queryParameters) : undefined
  };

[.....]

}

I think an approach could be:

/**
 * Create an 'SQL query' request
 * (using GET or POST request, depending on url size)
 */
function createRequest({ credentials, connection, query, opts = {}, queryParameters = null }) {
  const { abortController, ...otherOptions } = opts;

  const { apiVersion = API_VERSIONS.V2 } = credentials;

  const rawParams = {
    client: CLIENT,
    q: query?.trim(),
    ...otherOptions,
    ...(queryParameters && {queryParameters: JSON.stringify(queryParameters)})
  };

  if (apiVersion === API_VERSIONS.V3) {
    rawParams.access_token = credentials.accessToken;
  } else if (apiVersion === API_VERSIONS.V1 || apiVersion === API_VERSIONS.V2) {
    rawParams.api_key = credentials.apiKey;
  }

  const requestOpts = { ...otherOptions };
  if (abortController) {
    requestOpts['signal'] = abortController.signal;
  }

  // Get request
  const urlParamsForGet = Object.entries(rawParams).map(([key, value]) =>
    encodeParameter(key, value)
  );
  const getUrl = generateApiUrl({
    credentials,
    connection,
    parameters: urlParamsForGet
  });
  if (getUrl.length < REQUEST_GET_MAX_URL_LENGTH) {
    return getRequest(getUrl, requestOpts);
  }

  // Post request
  const urlParamsForPost =
    apiVersion === API_VERSIONS.V3
      ? [`access_token=${credentials.accessToken}`, `client=${CLIENT}`]
      : null;
  const postUrl = generateApiUrl({
    credentials,
    connection,
    parameters: urlParamsForPost
  });
  return postRequest(postUrl, rawParams, requestOpts);
}

@jantolg jantolg reopened this Nov 11, 2022
@Josmorsot
Copy link
Contributor

Hi @jantolg, is occurring with request through GET or POST? Can you share an example?

@jantolg
Copy link
Contributor

jantolg commented Nov 11, 2022

@Josmorsot is ocurring with POST requests. FYI: I'm using v1.4.5

 executeSQL({
    query: 'SELECT 1',
    credentials,
    connection: 'carto_dw',
  })

@Josmorsot
Copy link
Contributor

Josmorsot commented Nov 14, 2022

I've just tried with:

executeSQL({
    query: 'SELECT 1',
    credentials,
    connection: 'bqconn',
  });

GET => https://gcp-us-east1.api.carto.com/v3/sql/bqconn/query?client=carto-react&q=SELECT%201&queryParameters=%5B%5D&access_token=

Response =>
image

CURL => curl 'https://gcp-us-east1.api.carto.com/v3/sql/bqconn/query?client=carto-react&q=SELECT%201&queryParameters=%5B%5D&access_token=<TOKEN>' \ -H 'authority: gcp-us-east1.api.carto.com' \ -H 'accept: application/json' \ -H 'accept-language: en-GB,en-US;q=0.9,en;q=0.8' \ -H 'cache-control: no-cache' \ -H 'origin: https://127.0.0.1:3000' \ -H 'pragma: no-cache' \ -H 'referer: https://127.0.0.1:3000/' \ -H 'sec-ch-ua: "Google Chrome";v="107", "Chromium";v="107", "Not=A?Brand";v="24"' \ -H 'sec-ch-ua-mobile: ?0' \ -H 'sec-ch-ua-platform: "macOS"' \ -H 'sec-fetch-dest: empty' \ -H 'sec-fetch-mode: cors' \ -H 'sec-fetch-site: cross-site' \ -H 'user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/107.0.0.0 Safari/537.36' \ --compressed

It works fine for me (with v1.4.5 and v1.4.6). Can you try to reproduce with latest version and share a curl or create a repo with an example?

@Josmorsot
Copy link
Contributor

Fixed in v1.4.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants