Scroll api 5.0, 5.1, 5.2 throwing error against elastic search 5.3+

[kjg]

When I use the 5.0, 5.1, 5.2 scroll API from elasticsearch-js 13.1.1 or 13.2.0 against elasticsearch versions 5.3 and up I get the error

Error: [illegal_argument_exception] Failed to parse request body

In this case the request body is just a plain string which is the scroll id, not a json payload of form {scrollId: scrollId}

Does elasticsearch 5.3 not allow for requests via API 5.0, 5.1, 5.2, or do those API definitions need to be updated, or any idea what I might be doing wrong here?

[kjg]

Wait, is the apiVersion we set in the client supposed to always match the version of elasticsearch you're connecting to? If so, does upgrading elasticsearch always require a version bump in the config of the client code and downtime of the app?

[kjg]

As far as I can tell, the string only body was last in the docs on version 1.7, all newer versions show using the json body.

Maybe all the api specs need to be updated then? Do we need to update

• elasticsearch-js/src/lib/apis/5_0.js

  Lines 5715 to 5740 in e2897da

  	api.scroll = ca({
  	params: {
  	scroll: {
  	type: 'time'
  	},
  	scrollId: {
  	type: 'string',
  	name: 'scroll_id'
  	}
  	},
  	urls: [
  	{
  	fmt: '/_search/scroll/<%=scrollId%>',
  	req: {
  	scrollId: {
  	type: 'string'
  	}
  	}
  	},
  	{
  	fmt: '/_search/scroll'
  	}
  	],
  	paramAsBody: 'scrollId',
  	method: 'POST'
  	});

• elasticsearch-js/src/lib/apis/5_1.js

  Lines 5848 to 5873 in e2897da

  	api.scroll = ca({
  	params: {
  	scroll: {
  	type: 'time'
  	},
  	scrollId: {
  	type: 'string',
  	name: 'scroll_id'
  	}
  	},
  	urls: [
  	{
  	fmt: '/_search/scroll/<%=scrollId%>',
  	req: {
  	scrollId: {
  	type: 'string'
  	}
  	}
  	},
  	{
  	fmt: '/_search/scroll'
  	}
  	],
  	paramAsBody: 'scrollId',
  	method: 'POST'
  	});

• elasticsearch-js/src/lib/apis/5_2.js

  Lines 5848 to 5873 in e2897da

  	api.scroll = ca({
  	params: {
  	scroll: {
  	type: 'time'
  	},
  	scrollId: {
  	type: 'string',
  	name: 'scroll_id'
  	}
  	},
  	urls: [
  	{
  	fmt: '/_search/scroll/<%=scrollId%>',
  	req: {
  	scrollId: {
  	type: 'string'
  	}
  	}
  	},
  	{
  	fmt: '/_search/scroll'
  	}
  	],
  	paramAsBody: 'scrollId',
  	method: 'POST'
  	});

to match

elasticsearch-js/src/lib/apis/5_3.js

Lines 5836 to 5864 in e2897da

	api.scroll = ca({
	params: {
	scroll: {
	type: 'time'
	},
	scrollId: {
	type: 'string',
	name: 'scroll_id'
	}
	},
	urls: [
	{
	fmt: '/_search/scroll/<%=scrollId%>',
	req: {
	scrollId: {
	type: 'string'
	}
	}
	},
	{
	fmt: '/_search/scroll'
	}
	],
	paramAsBody: {
	param: 'scrollId',
	body: 'scroll_id'
	},
	method: 'POST'
	});

[kjg]

After much digging, I think this is the change that needs to be made (followed by a npm run generate)

diff --git i/scripts/generate/overrides.js w/scripts/generate/overrides.js
index fd8aa9c..2607852 100644
--- i/scripts/generate/overrides.js
+++ w/scripts/generate/overrides.js
@@ -252,7 +252,7 @@ function (spec) {
     }
   },
   {
-    version: '>=5.3',
+    version: '>=2.0',
     paramAsBody: {
       scroll: {
         param: 'scrollId',

What do you think @spalger? Do you happen to remember why you had it as 5.3+ before

[spalger]

I did that as 5.3+ because elastic/elasticsearch#22691 went into 5.3, but that's unfortunate that this doesn't work in 5.0+... I really need to get the functional tests for the client up and running again

[spalger]

Are you sure that should be >=2.0? Wouldn't >=5.0 be more appropriate?

[kjg]

@spalger I went with >=2.0 because the 1.7 docs show the body as just the requestId value

curl -XGET  'localhost:9200/_search/scroll?scroll=1m'   \
     -d       'c2Nhbjs2OzM0NDg1ODpzRlBLc0FXNlNyNm5JWUc1' 

and the 2.0 and up docs all support the json version

curl -XGET  'localhost:9200/_search/scroll'  -d'
{
    "scroll" : "1m", 
    "scroll_id" : "c2Nhbjs2OzM0NDg1ODpzRlBLc0FXNlNyNm5JWUc1" 
}
'

And the note about for backwards compatibly scroll_id can be passed in the request body was removed in the 5.0 docs

My opinion is 2.0 -> 2.4 should use the json body version instead of the backwards compatible version since they support both styles, but you are obviously much closer to the project, so I'll defer to your expertise.

Would you like me to make the PR for this change? If so should I use >=2.0 or >=5.0 after the above consideration? And also, should I include the new output of npm run generate in the PR or does that always get updated after merge into master?

Thanks so much for taking a look at this!

[spalger]

Nice, sounds like a valid reason to me! A PR would be very helpful, and >=2.0 sounds great

[kjg]

Should I include the output of npm run generate in the PR or does that always get updated after merge into master?

[spalger]

@kjg I actually run it manually but like to keep it separate from the PRs. Thanks again!