Skip to content

Commit

Permalink
src: improve error message when ICU data cannot be initialized
Browse files Browse the repository at this point in the history
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 <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and ruyadorno committed Sep 28, 2023
1 parent 7c2060c commit 10a2ade
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 25 deletions.
11 changes: 8 additions & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
23 changes: 14 additions & 9 deletions src/node_i18n.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/node_i18n.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
30 changes: 18 additions & 12 deletions test/parallel/test-icu-data-dir.js
Original file line number Diff line number Diff line change
@@ -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/
});
}

0 comments on commit 10a2ade

Please sign in to comment.