【非同期処理】処理の順番が思い通りになりません。

Question

前提・実現したいこと

javascriptでTwitter管理システムを作っています。
投稿の機能を実装中に以下の問題が発生しました

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

scheduledPost関数ないの全ての処理が終わってから、returnで結果を返したいが、処理が終わる前に、returnが実行されている。原因は、「executionCheckedProcessingsData.forEach」の部分だと考えています。

呼び出した関数から { success: [], fail: [ '22時08分' ], error: [] }
error 19時22分 [ { code: 187, message: 'Status is a duplicate.' } ]
error 22時15分 [ { code: 187, message: 'Status is a duplicate.' } ]
error 19時40分 [ { code: 187, message: 'Status is a duplicate.' } ]
error 19時11分 [ { code: 187, message: 'Status is a duplicate.' } ]
error 19時30分 [ { code: 187, message: 'Status is a duplicate.' } ]
error 22時05分 [ { code: 187, message: 'Status is a duplicate.' } ]
error 00時00分 [ { code: 187, message: 'Status is a duplicate.' } ]
error 22時16分 [ { code: 187, message: 'Status is a duplicate.' } ]

該当のソースコード

scheduledPost.js

const scheduledPost = async (req) => {
  // 【質問】外部モジュールのimportは、自作関数の中に書くか、外に書くか？（2021/03/23）
  const tweet = require("../mainFunctions/tweet");
  const executionCheck = require("./executionCheck");

  const executionCheckedProcessingsData = await executionCheck(req);

  const result = {
    success: [],
    fail: [],
    error: [],
  };

  executionCheckedProcessingsData.forEach(async (data) => {
    if (data.execution) {
      await tweet(data.tweet)
        .then(() => {
          console.log("success", data.processingName);
          result.success.push(data.processingName);
        })
        .catch((error) => {
          console.log("error", data.processingName, error);
          result.error.push(data.processingName, error);
        });
    } else {
      result.fail.push(data.processingName);
    }
  });
  return result;
};

scheduledPost("hoge").then((result) => {
  console.log("呼び出した関数から", result);
});

module.exports = scheduledPost;

tweet.js

const Twitter = require("twitter");
const oAuth = require("../keys/authKeys.json");

const tweet = async (postContent) => {
  const client = new Twitter(oAuth);

  const result = await client.post("statuses/update", { status: postContent });

  return result;
};

module.exports = tweet;

executionCheck.js

const executionCheck = async (req) => {
  const getJapanTime = require("../otherFunctions/getJapanTime");
  const fetchProcessingsData = require("../mainFunctions/fetchProcessingsData");

  const JapanTime = getJapanTime();
  const processingsData = await fetchProcessingsData();

  const executionCheckedProcessingsData = await processingsData.map((data) => {
    if (req) {
      data["execution"] = data.switch === "ON";
      return data;
    } else {
      data["execution"] = data.switch === "ON" && data.startTime === JapanTime;
      return data;
    }
  });

  return executionCheckedProcessingsData;
};

module.exports = executionCheck;

試したこと

・非同期処理の問題だと思い、aysnc/awaitやthenメソッドなどを使ってみましたがうまくいきませんでした。

補足情報（FW/ツールのバージョンなど）

node.js v12
twitter 1.7.1

Comments

[int32_t]

配列 executionCheckedProcessingsData に対する twee() 処理は、配列の全要素を並列に処理したいですか、配列の要素を1つずつ処理したいですか?

[ryota002]

全てに対して処理が行われ、結果がresultに格納されれば、どちらでも構いません。
並列の方が処理速度が早いですか？
また、並列か1つずつで書き方はどのように変わりますか？

Answer 1

JS上で非同期処理をどの順番でやっているかは厳密に決められています。
いずれですが一度きっちり学習するべきですが、簡単に説明しておきます。

JSにはイベント置き場というものがあり、
「達成条件・実行して欲しい処理を関数で包んだもの」を1セットでイベント登録を行います。
イベント登録が行われた際、JSは「わかった、後で達成されたら確認して実行しておくよ」とスルーします。

console.log(1);
setTimeout(
  () => console.log(2),
  0
);
console.log(3); 

setTimeout(fn, time)はtimeミリ秒待ってから実行するイベント登録です。
0ミリ秒なのでその場で即条件を達成していますが、この関数は何時実行されるのでしょうか？
正解はイベント登録だけ行って関数の実行をせず一度下まで通しで実行する。

その後にイベントループという達成したイベントを巡回して探す処理がJSの裏側で起こり、
0ミリ秒経過して達成している関数を拾い上げてconsole.log(2)を実行します。

従って出力結果は必ず1 -> 3 -> 2の順番になります。

---

今回のケースはPromiseを使っていますが、
Promiseもイベント登録です。

JSの非同期処理のクソな部分として
コールバック地獄でコードが無限にネストするというのが挙げられますが、

Promiseはあらゆるオブジェクト指向のテクニックを使って、
コールバック地獄をメソッドチェーンで記述して解決出来るようにしただけの代物です。
別にPromiseを使ったから非同期処理から解き放たれる訳ではなく、
JSのイベント登録→イベント達成→関数実行という流れに逆らっては居ません。

async/await構文はいわゆる糖衣構文で、
Promiseの.then(fn)を使っているだけです。
イベント登録の先で拾うよう指示を出しているだけです。

---

以上を踏まえてコードを添削します。

scheduledPost.js

// 外部モジュールのimportは、どうしても実現出来ない場合を除いて常にファイルの先頭で行う
const tweet = require("../mainFunctions/tweet");
const executionCheck = require("./executionCheck");

const scheduledPost = async req => {
  const executionCheckedProcessingsData = await executionCheck(req);

  const result = {
    success: [],
    fail: [],
    error: [],
  };

  // forEachはPromiseの束縛が切れて全てを即時実行するので、for...ofを使う
  for (const {execution, tweet: text, processingName} of executionCheckedProcessingsData) {
    // ガード節でネストを減らす
    if (!execution) {
      result.fail.push(processingName);
      continue;
    }
    try {
      await tweet(text);
      console.log("success", processingName);
      result.success.push(processingName);
    } catch (e) {
      console.error("error", processingName, e);
      result.error.push(processingName, e);
    }
  }

  return result;
};

// TODO:テストコード？後で削除する
scheduledPost("hoge").then(result => {
  console.log("呼び出した関数から", result);
});

module.exports = scheduledPost;

executionCheck.js

const getJapanTime = require("../otherFunctions/getJapanTime");
const fetchProcessingsData = require("../mainFunctions/fetchProcessingsData");

// 変数は1個でも減らしたほうが可読性が高くなる
module.exports = async req => {
  const JapanTime = getJapanTime();
  const processingsData = await fetchProcessingsData();

  return processingsData.map(data => {
    ...data,
    execution: req
      ? data.switch === "ON"
      : data.switch === "ON" && data.startTime === JapanTime
  });
};

Comments

[ryota002]

こんなにご丁寧に大変ありがとうございます。
ページの更新前に、他の方の内容で処理がうまくいったので、貴方の回答に気づく前にベストアンサーを決めてしまいました。
回答いただいた内容は、とても勉強になる内容でした。
現在の自分には、少し難しく、深く勉強する必要があるなと感じました。
回答いただいた内容を参考に勉強を進めていきます。
フォローさせていただいたので、勉強を進めていく中で躓くところがありましたら、また質問させてください。
ご丁寧に、ご回答いただきありがとうございました。

Answer 2

forEachの中身をasyncで書いても、外側のasyncとは関連せずに進んでしまいます。

「非同期処理をすべて待ちたい」なら、mapでPromiseを作った上で、await Promise.allとしましょう（参考）。

Comments

[maisumakun]

これは「全部並列で実行する」形となります。

[ryota002]

上手くいきました！
ありがとうございました！