JavaScript(Node.js)の非同期通信でリクエストを分割して処理をしたい

Question

前提・実現したいこと

非同期通信やpromiseについての理解が乏しいので質問させてください。
取得したjanコードを元にkeepaのapiで他サイトの価格を引っ張ってくるツールを制作しようとしています。
元のjanコードの量は場合により1000,2000個のような膨大な数になる可能性があり（ここではdataListX）、keepaのapiではリクエスト上限が100なのでjanコードを100ずつ区切ってapiを叩いて結果を取得して整形..と繰り返していきたいのですが、
ここのコードの26行目以下（await sendData(chunk).then((response)=> { ..の部分）でレスポンスがないまま先に進んでしまいエラーになります。

promise周りのみならず、書き方がまずい部分もあればご教授いただきたいです。
よろしくおねがいします。

発生している問題・エラーメッセージ

(node:220) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'length' of undefined   
(node:220) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:220) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

該当のソースコード

const puppeteer = require('puppeteer');
const fs = require('fs');
const rp = require('request-promise'); 
const $ = require('jQuery');
const _ = require( 'lodash' );

var dataListY = []

let dataListX = [
  ["https://www.netsea.jp/shop/25774/UAG-SFPRO4-BLK?cx_search=score","4988481755953","ビギナー拒否設定のため非公開"],
  ["https://www.netsea.jp/shop/25774/UAG-SFPRO4-CBT?cx_search=score","4988481755977","ビギナー拒否設定のため非公開"],
  ["https://www.netsea.jp/shop/25774/CRT-PFNG116W?cx_search=score","4969887294314","ビギナー拒否設定のため非公開"],
  ...
];


//リクエストを作成
let productCodes = []; //#2 100個まで
for (var i = 0; i < dataListX.length; i++) {
  productCodes.push(dataListX[i][1]);
}

async function dataFlow(){
  for ( const chunk of _.chunk( productCodes, 100 ) ) {
    // 100ずつの配列をsendData関数に渡す
    await sendData(chunk).then((response)=> {
      if(response){
      let products = response.products; //response.products にそれぞれの商品のデータが配列で入ってる
      console.log(products.length); //=>3 3つリクエストしたのでレスポンスも3つ
      for(var i=0;i<products.length;i++){
        var product = products[i];
        let itemDetail = [];
        if(product.eanList) {
          itemDetail.push(String(product.eanList)); // EAN(JAN)
        } else {
          itemDetail.push('');
        }
        itemDetail.push(product.title); // 商品名
        if(product.csv[0]) { // Amazon本体の最新の出品価格
          itemDetail.push(product.csv[0][product.csv[0].length-1]);
        } else{
          itemDetail.push('');
        }
        if(product.csv[1]) { // 新品の最新の出品価格(最安値)
          itemDetail.push(product.csv[1][product.csv[1].length-1]);
        } else {
          itemDetail.push('');
        }
        // console.log('---------------------------------------------------------------------------');
        dataListY.push(itemDetail);
        console.log(dataListY);
        for(var i=0;i<dataListX.length;i++){
          for(var ii=0;ii<dataListY.length;ii++){
            let searchItem = dataListY.vlookup(dataListY[ii][0], 0);
            let pushItem1 = dataListY.vlookup(dataListY[ii][0], 1);
            let pushItem2 = dataListY.vlookup(dataListY[ii][0], 2);
            let pushItem3 = dataListY.vlookup(dataListY[ii][0], 3);
            if (dataListX[i][1] === searchItem) {
              dataListX[i].push(pushItem1, pushItem2, pushItem3);
            }
          }
        }
      }
    }
  })
}
dataFlow();

async function sendData(chunk) {
  const options = {
    url : 'https://api.keepa.com/product',
    method : 'POST',
    form : { 
        key : xxxxxxxxxxxxxx, 
        code : chunk.join(','), 
        domain : '5', 
    },
    gzip: true, 
    headers : {
      'Content-Type':'application/json',
      'Connection' : 'keep-alive' 
    },
    json : true,
    resolveWithFullResponse: true
  }
  const response = rp(options);
  if(response) return response;
}

Answer 1

パッと見でダメそうな箇所はいくつかありますが、
順番にいってみましょうか。

TypeError: Cannot read property 'length' of undefined

JSではこのエラー文は頻出です。
即何処を直せば良いか理解出来るようになりましょう。

まず、JSはオブジェクト指向言語なので
value.propertyという風にドットプロパティ名という記述で
メソッドやプロパティを呼び出す事ができます。

しかし、JSではプロパティの所持が許されない例外の値が２つ存在し、
アクセスを試みただけでエラーで落ちます。

• null
• undefined

当てずっぽうですが、
恐らくこれで間違いないでしょう。

      let products = response.products; //response.products にそれぞれの商品のデータが配列で入ってる
      console.log(products.length); //=>3 3つリクエストしたのでレスポンスも3つ

responseが何者かはチェックしていますが、
productsというプロパティが何者かをチェックしていません。
つまり、こういう流れでエラーになっているのでしょう。

• response: 何か入っている
• response.products: キーが存在しない、undefined
• response.products.length: undefinedからプロパティ参照を行った為にエラー

なぜこいつが怪しいかという話ですが、
sendData関数の戻り値はかなり怪しいですね。
本当にproductsキーに値が入っている事は保証されるんですか？

デバッグをしながら少しずつ進めていってください。

---

ではリファクタリング案に関して見ていきましょう。

        for(var i=0;i<dataListX.length;i++){
          for(var ii=0;ii<dataListY.length;ii++){
            let searchItem = dataListY.vlookup(dataListY[ii][0], 0);
            let pushItem1 = dataListY.vlookup(dataListY[ii][0], 1);
            let pushItem2 = dataListY.vlookup(dataListY[ii][0], 2);
            let pushItem3 = dataListY.vlookup(dataListY[ii][0], 3);
            if (dataListX[i][1] === searchItem) {
              dataListX[i].push(pushItem1, pushItem2, pushItem3);
            }
          }
        }

なんでこいつが毎ループで実行されるんですか？
計算量が爆発して死ぬ未来しか見えません。

更にdataListYって何者なんですか？
上の方にvar dataListY = []って書いてますよね？
JavaScriptのArrayが所持しているメソッドにvlookupというものはありません。

恐らく大幅に省かれた何かが原因でしょう。
よってこれに関してはノータッチとします。

次。

const rp = require('request-promise'); 
const $ = require('jQuery');

Node.jsにはDOMを扱う機能が入ってないので、
Node.js上でHTMLファイルをパースして情報を抜き出したいならば、
cheerioを使いましょう。

async function dataFlow(){
  for ( const chunk of _.chunk( productCodes, 100 ) ) {
    // 100ずつの配列をsendData関数に渡す
    await sendData(chunk).then((response)=> {
      if(response){
      let products = response.products; //response.products にそれぞれの商品のデータが配列で入ってる

if文の次の行、字下げ忘れですね。
なのに関数閉じる箇所は綺麗な階段になっている。

推測ですがコピペのしすぎでカッコの開き、閉じの対応が取れてないのでは？
見直してみてください。

そもそも1個のif文で何十行も中括弧({})で包むからネストが深くなって辛いんですよ。
リーダブルコードを読んだ上で、早期returnを採用する等した方が良いでしょう。
for文の中で使うならif (!response) continue;にするとか。

await sendData(chunk).then((response)=> {

これではawaitを使っている意味が1ミリもありません。
awaitにはPromiseオブジェクトを待つという意味があります。

これで不要なthenメソッドの呼び出しや、
関数を包んでの実行を省略するという意味で使います。
上記までを踏まえて書くとこんな感じになります。

async function dataFlow(){
  for ( const chunk of _.chunk( productCodes, 100 ) ) {
    const response = await sendData(chunk);
    if (!response) continue;
    let products = response.products; //response.products にそれぞれの商品のデータが配列で入ってる
    console.log(products.length); //=>3 3つリクエストしたのでレスポンスも3つ

    // 既存の流れ
  }
}

まぁまぁ綺麗になってきました。

書き方がまずい部分もあればご教授いただきたいです。

二次元配列はやめましょう。
配列の0番目はURLで、1番目にはIDが入ってて……
みたいな作りは後々忘れてコードの修正で死ぬので絶対に無しです。

貴方はdataListX[i][1]が何かを即答出来ますか？

似たような構造の繰り返しデータになる場合、
オブジェクトの配列が最も適しています。
情報量が多くなる場合は、YAMLファイル等の外部ファイルで管理するのも良い

const dataListX = [
  {
    url: "https://www.netsea.jp/shop/25774/UAG-SFPRO4-BLK?cx_search=score",
    productCode: "4988481755953",
    detail: "ビギナー拒否設定のため非公開",
  },
  {
    url: "https://www.netsea.jp/shop/25774/UAG-SFPRO4-CBT?cx_search=score",
    productCode: "4988481755977",
    detail: "ビギナー拒否設定のため非公開",
  },
  {
    url: "https://www.netsea.jp/shop/25774/CRT-PFNG116W?cx_search=score",
    productCode: "4969887294314",
    detail: "ビギナー拒否設定のため非公開",
  },
  ...
];

こう変更した威力は次項でわかります。

for (var i = 0; i <

なんで他の箇所ではfor...of使ってるのに、
いきなりforに戻るんですか？
どちらかに統一しましょう。

forかfor...ofのどちらを使うかの判断基準ですが、
iという添字自体に用事はなく、配列の中身の方に用事があるんですよね？
こういうiみたいな変数を状態変数と呼び、文章を読み書きする上で理解の妨げになるノイズです。
減らした方が偉いので減らしましょう。

配列やコレクションを元にループを回したい場合、
出来る限りfor...ofを利用しましょう。
前述の項目と合わせてこうなります。

const productCodes = []; // pushはconstでも可能
for (const {productCode} of dataListX) {
  productCodes.push(productCode);
}

// 因みにArray.prototype.mapを使うと1行で書ける
const productCodes = dataListX.map(it => it.productCode);

しかし、URLがそれぞれ異なるので
これを100個ひとまとめにしても無駄なのでは？

---

dataListYも同様ですね。
という訳で、上記を踏まえてざっくり書き直すとこうなります。

const dataListX = [
  {url: "https://www.netsea.jp/shop/25774/UAG-SFPRO4-BLK?cx_search=score", productCode: "4988481755953", detail: "ビギナー拒否設定のため非公開"},
  {url: "https://www.netsea.jp/shop/25774/UAG-SFPRO4-CBT?cx_search=score", productCode: "4988481755977", detail: "ビギナー拒否設定のため非公開"},
  {url: "https://www.netsea.jp/shop/25774/CRT-PFNG116W?cx_search=score", productCode: "4969887294314", detail: "ビギナー拒否設定のため非公開"},
  ...
];
const productCodes = dataListX.map(it => it.productCode);

async function dataFlow() {
  // 一度に動作させるfor文の粒度は小さくした方が良い
  // productsをまず受け取って配列を作る
  const products = [];
  for (const chunk of _.chunk(productCodes, 100)) {
    // 100ずつの配列をsendData関数に渡す
    const response = await sendData(chunk);
    if (!response) continue; // なんで結果が帰ってこないんだ、例外とかエラーとして通知する必要ありそう
    for (const p of response.products) {
      products.push(p);
    }
  }

  // 変数は使う直前で宣言すべし
  const dataListY = [];
  for (const product of products) {
    dataListY.push({
      title: product.title,
      ean: product.eanList ? String(product.eanList) : "",
      amznPrice: product.csv[0] ? _.last(product.csv[0]) : "",
      newPrice: product.csv[1] ? _.last(product.csv[1]) : "",
    });
  }

  // データリストの突き合わせは省略
}

Comments

[tax231]

ご返信が大変遅れました。
読み込んで修正とリファクタリングを行い、徐々に解決していきました。
大変勉強になりました。
TypeError: Cannot read property 'length' of undefined
こちらはsendDataのoptionでいらぬ設定をしていたことで起きたものでした。

[miyabi-sun]

お疲れ様です、原因特定まで出来て良かったです。
よく頑張りましたね。

オタクの超早口みたいな豪速球な回答だったので、
飲み込んでコードに反映するまでにかなり時間が掛かるだろうなとは思ってました。

余談ですが、ソースコードの書式の綺麗さを担保したい場合、
eslintを使ってみるのも良いでしょう。
「eslint vscode」という風に利用しているエディタとセットで検索すると、
エディタでJSファイルを開いた時に裏で解析しながら教えてくれるようになります。