Query parameters not correctly parsed

[itpropro]

The source code describes the type for the query parameters as being only string values

azure-functions-nodejs-worker/types/index.d.ts

Line 111 in d51a7b0

[name: string]: string;

which is not always the case.
The following query would result in a JavaScript object that consist of only strings:

GET https://functionapp.local/api/function01?param1=false&param2=100&param3=test

// req.query looks like this
{
  param1: 'false',
  param2: '100',
  param3: 'test',
}

// req.query should look like this
{
  param1: false,
  param2: 100,
  param3: 'test',
}

Repro steps

You can easily create a querystring by using e.g. the query-string library:
const testQs = queryString.stringify({param1: false, param2: 100, param3: 'test'});.
To get the correct results, it can then be parsed using the same library:
queryString.parse(testQs, {parseBooleans: true, parseNumbers: true});.

I couldn't find details on query parameter parsing on the Docs, things like supported types and array formats for query strings would really be helpful.

[ejizba]

I believe req.query is consistent with the default behavior of queryString.parse, since both parseBooleans and parseNumbers default to false. With that in mind, I would say our existing behavior is "correct" (aka this isn't a bug), but rather it sounds like a feature request to support those config options (parseBooleans and parseNumbers or something similar). Does that sound right?

[itpropro]

I believe req.query is consistent with the default behavior of queryString.parse, since both parseBooleans and parseNumbers default to false. With that in mind, I would say our existing behavior is "correct" (aka this isn't a bug), but rather it sounds like a feature request to support those config options (parseBooleans and parseNumbers or something similar). Does that sound right?

I think it should not be consistent with the behavior of querystring, but with the HTTP RFC. I don't know what the exact result would be with the built-in node qs module, but having to write custom code for every field that you want to contain another type than string for every single function (except you are using durable then it's only the starter function) can't be solution.

EDIT: I made a few checks and most of the query string parsing libraries I tested have this as optional and a few as default. query-string, qs and querystring have it as optional.
It would be helpful to have at least some configuration options exposed to include parsing of booleans/integers or some guidance how to best handle different data types to avoid code duplication.

[ejizba]

I think it should not be consistent with the behavior of querystring, but with the HTTP RFC.

I don't have a quick answer on this, but regardless it would be a breaking change so it probably wouldn't happen any time soon.

having to write custom code for every field that you want to contain another type than string for every single function ... can't be solution.

We're working on some new features that would make it easier for you to write code that would apply to all functions, which could help in this case. The feature is called invocation hooks and you could do something like this:

import { registerHook } from '@azure/functions-core';

registerHook('preInvocation', (context) => {
    // if context.req.query exists
    // for (const [key, value] of Object.entries(context.req.query))
    // try parse `value` as boolean
    // try parse `value` as number
});

NOTE: this feature is still rolling out in Azure and we have not published a types package for "@azure/functions-core" yet.

It would be helpful to have at least some configuration options exposed to include parsing of booleans/integers or some guidance how to best handle different data types to avoid code duplication.

I agree. The hooks mentioned above should help with code duplication, but we can look into providing specific options as well

[itpropro]

I understand, if it would help, I could create a PR to the docs with an example using the mentioned hooks.
For Durable Functions, would you agree, that something like a switch statement over the req.query object in the starter function would be a “good” way of solving this? If tes I would also add that to the PR.

[ejizba]

We're not ready to document the hooks feature yet. People can use hooks directly from the @azure/functions-core module, but the long-term recommended way of using hooks will be as a part of the new programming model which is a larger effort currently underway

cc @davidmrdavid for the question on durable functions. My quick answer is that if it works for you then yes it's "good" 🙂

[itpropro]

Are hooks now a fully documented and native ESM compatible feature @ejizba ? Since the last update, there was a lot of shifting towards nodeless edge worker environments for deployment.

[ejizba]

We switched to use a more standard Node.js type URLSearchParams for query in the new v4 model. With that in mind, I don't think we need to keep this issue open.

Hook support can be tracked here: https://github.com/Azure/azure-functions-nodejs-library/issues?q=is%3Aopen+is%3Aissue+label%3Ahooks