search

Esri / arcgis-rest-js

compact, modular JavaScript wrappers for the ArcGIS REST API
https://developers.arcgis.com/arcgis-rest-js/
Apache License 2.0
347 stars 120 forks source link

fix(arcgis-rest-request): node ReadStream encoding bug at 3.x #1034

Closed haoliangyu closed 1 year ago

haoliangyu commented 1 year ago

This PR addresses the issue https://github.com/Esri/arcgis-rest-js/issues/1033 at rest-js v3. The bug is related to the form-data library in the node environment. It requires to use an unofficial option to specify the stream length so that the streaming request can end properly (see https://github.com/Esri/arcgis-rest-js/issues/1033#issuecomment-1292016958).

I add a dataSize option to the addItemData() function so that the user can give a number for the stream data length.

PS: I suspect other functions accepting node ReadStream can have the same issue, but I don't have the availability to verify and fix each API.

PS2: I assume that no more than one stream is used in one API call.

PS3: I have to update the npm version to v7 in CI so that npm install won't throw an error.

haoliangyu commented 1 year ago

~The CI failed at the Chrome test stage because that required 100% test coverage for the code. But this change is for node environment. Is there a way to bypass the test coverage check for a certain block of code?~ fixed

dbouwman commented 1 year ago

For rest-js v3, we did not have semantic-release, so you actually want to PR this to master and then we manually run the release like we did in the olden times.

This PR is the last time we did a 3.x release.

I can run the scripts if you want. Just lmk via teams once this is merged.

tomwayson commented 1 year ago

Thanks @dbouwman, yeah, I just noticed that we don't have a .releaserc.json file that I believe would be required to run the release on maintenance branches.

I've updated the base, and will merge and run the scripts.

haoliangyu commented 1 year ago

@tomwayson Do you have any availability to release this as v3.5.1?

tomwayson commented 1 year ago

@haoliangyu this has been published as v.3.5.1