Proposal: Supporting User-Defined "Follow If" Conditions
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 1.6k
- Forks
- 237
- PR merge metrics
- No merged PRs in 30d
Description
I needed a bit more control over the follow-if logic today, so I created a small patch to enable the declaration of numerous user-defined conditions:
const options = {
follow_max: 5,
follow_if_all: [
// Only follow if the new endpoint is HTTP or HTTPS
( redirect, original ) => /^https?:/.test( redirect )
]
};
I'm okay with a redirect from HTTP to HTTPS, or the other way around, but I encountered a URL today that aimed to redirect from HTTPS to MAILTO. This would throw from within http.request, causing all sorts of problems for me.
I'd love to hear everybody's thoughts about this proposal.
Here is the diff that solved my problem:
diff --git a/node_modules/needle/lib/needle.js b/node_modules/needle/lib/needle.js
index a8a1627..509bb37 100644
--- a/node_modules/needle/lib/needle.js
+++ b/node_modules/needle/lib/needle.js
@@ -105,7 +105,10 @@ var defaults = {
follow_keep_method : false,
follow_if_same_host : false,
follow_if_same_protocol : false,
- follow_if_same_location : false
+ follow_if_same_location : false,
+
+ // custom
+ follow_if_all: []
}
var aliased = {
@@ -258,7 +261,7 @@ Needle.prototype.setup = function(uri, options) {
function check_value(expected, key) {
var value = get_option(key),
- type = typeof value;
+ type = Array.isArray( value ) ? 'array' : typeof value;
if (type != 'undefined' && type != expected)
throw new TypeError(type + ' received for ' + key + ', but expected a ' + expected);
@@ -290,6 +293,10 @@ Needle.prototype.setup = function(uri, options) {
config[key] = check_value('number', key);
})
+ keys_by_type(Array).forEach(function(key) {
+ config[key] = check_value('array', key);
+ })
+
// populate http_opts with given TLS options
tls_options.split(' ').forEach(function(key) {
if (typeof options[key] != 'undefined') {
@@ -511,6 +518,10 @@ Needle.prototype.should_follow = function(location, config, original) {
if (config.follow_if_same_protocol && !matches('protocol'))
return false; // procotol does not match, so not following
+ if ( config.follow_if_all && !config.follow_if_all.every( o => o( location ) ) ) {
+ return false; // One of the user-provided conditions were not met
+ }
+
return true;
}
This issue body was partially generated by patch-package.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with node_modules/needle/lib/needle.js, especially Needle.prototype.setup and Needle.prototype.should_follow, and review the existing follow_if_* options. Check how redirect handling and option validation are tested; done means the proposed follow_if_all behavior works without breaking existing follow rules and has regression coverage.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- api
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100