src: improve error message when ICU data cannot be initialized

Previously when we fail to initialize ICU data, the error message is ``` could not initialize ICU (check NODE_ICU_DATA or --icu-data-dir parameters) ``` This patch updates it to something similar to: ``` U_FILE_ACCESS_ERROR: Could not initialize ICU. Check the directory specified by NODE_ICU_DATA or --icu-data-dir contains icudt73l.dat and it's readable ``` Where the expected data file name is the same as U_ICUDATA_NAME. PR-URL: #49666 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Sep 28, 2023 · 10a2ade · 10a2ade
1 parent 7c2060c
commit 10a2ade
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 25 deletions.
diff --git a/src/node.cc b/src/node.cc
@@ -916,9 +916,14 @@ static ExitCode InitializeNodeWithArgsInternal(
 
     // Initialize ICU.
     // If icu_data_dir is empty here, it will load the 'minimal' data.
-    if (!i18n::InitializeICUDirectory(per_process::cli_options->icu_data_dir)) {
-      errors->push_back("could not initialize ICU "
-                        "(check NODE_ICU_DATA or --icu-data-dir parameters)\n");
+    std::string icu_error;
+    if (!i18n::InitializeICUDirectory(per_process::cli_options->icu_data_dir,
+                                      &icu_error)) {
+      errors->push_back(icu_error +
+                        ": Could not initialize ICU. "
+                        "Check the directory specified by NODE_ICU_DATA or "
+                        "--icu-data-dir contains " U_ICUDATA_NAME ".dat and "
+                        "it's readable\n");
       return ExitCode::kInvalidCommandLineArgument;
     }
     per_process::metadata.versions.InitializeIntlVersions();

diff --git a/src/node_i18n.cc b/src/node_i18n.cc
@@ -54,20 +54,21 @@
 #include "util-inl.h"
 #include "v8.h"
 
-#include <unicode/utypes.h>
 #include <unicode/putil.h>
+#include <unicode/timezone.h>
 #include <unicode/uchar.h>
 #include <unicode/uclean.h>
+#include <unicode/ucnv.h>
 #include <unicode/udata.h>
 #include <unicode/uidna.h>
-#include <unicode/ucnv.h>
-#include <unicode/utf8.h>
-#include <unicode/utf16.h>
-#include <unicode/timezone.h>
 #include <unicode/ulocdata.h>
+#include <unicode/urename.h>
+#include <unicode/ustring.h>
+#include <unicode/utf16.h>
+#include <unicode/utf8.h>
+#include <unicode/utypes.h>
 #include <unicode/uvernum.h>
 #include <unicode/uversion.h>
-#include <unicode/ustring.h>
 
 #ifdef NODE_HAVE_SMALL_ICU
 /* if this is defined, we have a 'secondary' entry point.
@@ -569,8 +570,7 @@ ConverterObject::ConverterObject(
   }
 }
 
-
-bool InitializeICUDirectory(const std::string& path) {
+bool InitializeICUDirectory(const std::string& path, std::string* error) {
   UErrorCode status = U_ZERO_ERROR;
   if (path.empty()) {
 #ifdef NODE_HAVE_SMALL_ICU
@@ -583,7 +583,12 @@ bool InitializeICUDirectory(const std::string& path) {
     u_setDataDirectory(path.c_str());
     u_init(&status);
   }
-  return status == U_ZERO_ERROR;
+  if (status == U_ZERO_ERROR) {
+    return true;
+  }
+
+  *error = u_errorName(status);
+  return false;
 }
 
 void SetDefaultTimeZone(const char* tzid) {

diff --git a/src/node_i18n.h b/src/node_i18n.h
@@ -38,7 +38,7 @@
 namespace node {
 namespace i18n {
 
-bool InitializeICUDirectory(const std::string& path);
+bool InitializeICUDirectory(const std::string& path, std::string* error);
 
 void SetDefaultTimeZone(const char* tzid);
 

diff --git a/test/parallel/test-icu-data-dir.js b/test/parallel/test-icu-data-dir.js
@@ -1,27 +1,33 @@
 // Flags: --expose-internals
 'use strict';
 const common = require('../common');
+const { spawnSyncAndExit } = require('../common/child_process');
 const { internalBinding } = require('internal/test/binding');
-const os = require('os');
 
 const { hasSmallICU } = internalBinding('config');
 if (!(common.hasIntl && hasSmallICU))
   common.skip('missing Intl');
 
-const assert = require('assert');
-const { spawnSync } = require('child_process');
-
-const expected =
-    'could not initialize ICU (check NODE_ICU_DATA or ' +
-    `--icu-data-dir parameters)${os.EOL}`;
-
 {
-  const child = spawnSync(process.execPath, ['--icu-data-dir=/', '-e', '0']);
-  assert(child.stderr.toString().includes(expected));
+  spawnSyncAndExit(
+    process.execPath,
+    ['--icu-data-dir=/', '-e', '0'],
+    {
+      status: 9,
+      signal: null,
+      stderr: /Could not initialize ICU/
+    });
 }
 
 {
   const env = { ...process.env, NODE_ICU_DATA: '/' };
-  const child = spawnSync(process.execPath, ['-e', '0'], { env });
-  assert(child.stderr.toString().includes(expected));
+  spawnSyncAndExit(
+    process.execPath,
+    ['-e', '0'],
+    { env },
+    {
+      status: 9,
+      signal: null,
+      stderr: /Could not initialize ICU/
+    });
 }