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
[http-request] : Can't set payload handling when method is passed in via msg.method
#3957
Comments
I have an example flow which I can share by forum DM if necessary. |
@Paul-Reed could you share a minimal flow here please? Ta. |
No, because it contains my api key/secret, but I can DM it to you, for your personal attention. EDIT - Now DM'd |
Issue confirmed.
@dceejay my preference would be to permit the payload handling option (for GET requests) to be set by the My reasoning is there may be only 1 HTTP Request Node on a flow, used for all GET/POST/ETC using My current solution would be to replace the at lines 203~205 ... with a "per msg" (on the fly) determination... this.on("input",function(msg,nodeSend,nodeDone) {
// ...
// ... snipped for brevity ---
// ...
// ...
let paytoqs = false
let paytobody = false
if (msg.method && n.method && (n.method === "use")) {
method = msg.method.toUpperCase(); // use the msg parameter
// If the method is set to GET (by msg.method), we need to check what to do with the payload
if (method === "GET") {
if (msg.payloadToQuerystring === true) {
paytoqs = true
} else if (msg.payloadToBody === true) {
paytobody = true
}
}
} else {
// use UI options (original code)
if (n.paytoqs === true || n.paytoqs === "query") {
paytoqs = true
} else if (n.paytoqs === "body") {
paytobody = true
}
} Thoughts please Dave? |
Makes complete sense to include it in the message if (method === "GET") {
switch (msg.payloadHandling.toLowerCase()) {
case 'qs':
paytoqs = true
break
case 'body':
paytobody = true
break
}
} else {
...
} Could make it easier to add more options in the future, using just one key to dictate handling. |
@marcus-j-davies I like the single msg property idea - it does simplify things somewhat. But for compatibility reasons, I would suggest the value options are
The built in docs would not bother to mention the let paytoqs = false
let paytobody = false
let payloadHandling = n.paytoqs
if (msg.method && n.method && (n.method === "use")) {
method = msg.method.toUpperCase(); // use the msg parameter
payloadHandling = msg.payloadHandling
}
// Test payloadHandling to see if it has been set
if (payloadHandling === true || payloadHandling === "query") {
paytoqs = true
} else if (payloadHandling === "body") {
paytobody = true
} NOTE: I realise I am not checking the method is a GET but it doesnt matter since that is checked further down in the code (paytoqs / paytobody are only ever used for GET requests) |
Sounds sensible to me... no objections here. |
Current Behavior
When
- set by msg.method -
is used for the method, it hides the payload handling dropdown on the node config editor.Now then, If the user does not first choose the payload handling option prior to selecting
- set by msg.method -
The users payload is ignored, i.e for GET requests, the payload is not passed in as a query string, as the default is to ignore it.
The user has to first set it back to GET, change the payload handling option, then set it back to
- set by msg.method -
.The option is there initially, but this could trip up new users.
Expected Behavior
Iether allow payload handling options to be include in the message, or to not hide the option
Steps To Reproduce
- set by msg.method -
Example flow
No response
Environment
Discussed here:
https://discourse.nodered.org/t/http-request-node-returning-error
The text was updated successfully, but these errors were encountered: