idobatter/node-win32ole

Chokes on variants which are null

Opened this issue · 2 comments

node-win32ole exits with an error when trying to retrieve a value which is null. I can only reproduce this problem with ADO, for which being able to use null values is pretty critical.

Example:

var win32ole = require('win32ole');
var fs = require('fs');

// Create a simple database (unless we already have)

var sFolder = __dirname + "/TextFileDb"
var sFilename = __dirname + "/TextFileDb/myTable.csv"

if (fs.existsSync(sFolder) == false) fs.mkdirSync(sFolder);
if (fs.existsSync(sFilename) == false) fs.writeFileSync(sFilename, "description,thing\r\nHere is a null value:,");

// Open our database

var sConnectionString = 'Provider=Microsoft.Jet.OLEDB.4.0;Data Source=' + sFolder + ';' +
                        'Extended Properties="text;HDR=Yes;FMT=Delimited"';

var db = win32ole.client.Dispatch('ADODB.Connection');
db.Open(sConnectionString);

var rs = win32ole.client.Dispatch('ADODB.Recordset');
rs.Open("select * from myTable.csv", db);

// Attempt to print the contents of our database

rs.MoveFirst();

while(rs.EOF == false)
{
    console.log(rs.Fields(0).Value.toString());
    console.log(rs.Fields(1).Value.toString());

    rs.MoveNext();
}

rs.Close();
db.Close();

Output:

Here is a null value:
ASSERT in 00000000 module ..\src\v8variant.cc(93) @node_win32ole::V8Variant::Cre
ateOCVariant: !v->IsNull()
error: The operation completed successfully.

D:\git\nodejsplay\test.js:31
    console.log(rs.Fields(1).Value.toString());
                                   ^
TypeError: node_win32ole::V8Variant::OLEFlushCarryOver can't access to V8Variant
 (null OCVariant)
    at Object.<anonymous> (D:\git\nodejsplay\test.js:31:36)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:901:3

For reference here is some equivalent PHP code:

<?php
// Skip database creation as the node.js script has already done that successfully

// Open our database

$sConnectionString = 'Provider=Microsoft.Jet.OLEDB.4.0;Data Source=./TextFileDb;' .
                     'Extended Properties="text;HDR=Yes;FMT=Delimited"';

$db = new COM('ADODB.Connection');
$db->Open($sConnectionString);

$rs = new COM('ADODB.Recordset');
$rs->Open("select * from myTable.csv", $db);

// Attempt to print the contents of our database

$rs->MoveFirst();

while($rs->EOF == false)
{
    echo($rs->Fields(0)->Value . "\r\n");
    echo($rs->Fields(1)->Value . "\r\n");

    $rs->MoveNext();
}

$rs->Close();
$db->Close();
?>

Which produces the expected output:

Here is a null value:

Thank you for fix bug. Please send me new Pull Request.
But I am afraid of that to replace to 'return new OCVariant();' may cause new problem at the other scene.

Apparently you have already pulled this. (GitHub is confusing sometimes!)

Any code for which 'return new OCVariant();' causes a problem would previously have asserted. So at worst I have replaced one problem with another. However I'd hope to do better than that. You know the code better than me so are more likely to be able to come up with an appropriate fix, but if you explain more about the potential danger I may be able to help.

Thank you for node-win32ole, and for your interest in the issues I have experienced while using it.