Skip to content

Commit 8f91110

Browse files
committed
Expose libgit2 error code to clients when a promise fails
When an API function of libgit2 fails within a promise, the promise is rejected with a JavaScript Error object that wraps the recorded error message in giterr_last(). However, the original non-zero return code of the API function is not exposed to the caller of the NodeGit API. This means that NodeGit clients cannot easily identify what the cause of an error is outside of parsing the Error object's message. This is an extremely volatile way of determining the cause as libgit2 may choose to alter the wording of the message at any time. To solve the problem above, the returned JavaScript Error object now esposes the libgit2 error code via its `errno` property.
1 parent 3e06530 commit 8f91110

5 files changed

Lines changed: 54 additions & 4 deletions

File tree

generate/templates/manual/patches/convenient_patches.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,15 @@ void GitPatch::ConvenientFromDiffWorker::HandleOKCallback() {
9595
}
9696

9797
if (baton->error) {
98+
Local<v8::Object> err;
99+
if (baton->error->message) {
100+
err = Nan::Error(baton->error->message)->ToObject();
101+
} else {
102+
err = Nan::Error("Method convenientFromDiff has thrown an error.")->ToObject();
103+
}
104+
err->Set(Nan::New("errno").ToLocalChecked(), Nan::New(baton->error_code));
98105
Local<v8::Value> argv[1] = {
99-
Nan::Error(baton->error->message)
106+
err
100107
};
101108
callback->Call(1, argv);
102109
if (baton->error->message)

generate/templates/manual/revwalk/fast_walk.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,15 @@ void GitRevwalk::FastWalkWorker::HandleOKCallback()
8888
{
8989
if (baton->error)
9090
{
91+
Local<v8::Object> err;
92+
if (baton->error->message) {
93+
err = Nan::Error(baton->error->message)->ToObject();
94+
} else {
95+
err = Nan::Error("Method fastWalk has thrown an error.")->ToObject();
96+
}
97+
err->Set(Nan::New("errno").ToLocalChecked(), Nan::New(baton->error_code));
9198
Local<v8::Value> argv[1] = {
92-
Nan::Error(baton->error->message)
99+
err
93100
};
94101
callback->Call(1, argv);
95102
if (baton->error->message)

generate/templates/manual/revwalk/file_history_walk.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,15 @@ void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback()
289289
}
290290

291291
if (baton->error) {
292+
Local<v8::Object> err;
293+
if (baton->error->message) {
294+
err = Nan::Error(baton->error->message)->ToObject();
295+
} else {
296+
err = Nan::Error("Method fileHistoryWalk has thrown an error.")->ToObject();
297+
}
298+
err->Set(Nan::New("errno").ToLocalChecked(), Nan::New(baton->error_code));
292299
Local<v8::Value> argv[1] = {
293-
Nan::Error(baton->error->message)
300+
err
294301
};
295302
callback->Call(1, argv);
296303
if (baton->error->message)

generate/templates/partials/async_function.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,15 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() {
155155
callback->Call(2, argv);
156156
} else {
157157
if (baton->error) {
158+
Local<v8::Object> err;
159+
if (baton->error->message) {
160+
err = Nan::Error(baton->error->message)->ToObject();
161+
} else {
162+
err = Nan::Error("Method {{ jsFunctionName }} has thrown an error.")->ToObject();
163+
}
164+
err->Set(Nan::New("errno").ToLocalChecked(), Nan::New(baton->error_code));
158165
Local<v8::Value> argv[1] = {
159-
Nan::Error(baton->error->message)
166+
err
160167
};
161168
callback->Call(1, argv);
162169
if (baton->error->message)

test/tests/merge.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,4 +1298,26 @@ describe("Merge", function() {
12981298
assert.ok(repository.isDefaultState());
12991299
});
13001300
});
1301+
1302+
it("can retrieve error code on if common merge base not found", function() {
1303+
var repo;
1304+
return NodeGit.Repository.open(local("../repos/workdir"))
1305+
.then(function(r) {
1306+
repo = r;
1307+
return repo.getCommit("4bd806114ce26503c103c85dcc985021951bbc18");
1308+
})
1309+
.then(function(commit) {
1310+
return commit.getParents(commit.parentcount());
1311+
})
1312+
.then(function(parents) {
1313+
return NodeGit.Merge.base(repo, parents[0], parents[1])
1314+
.then(function() {
1315+
return Promise.reject(new Error(
1316+
"should not be able to retrieve common merge base"));
1317+
}, function(err) {
1318+
assert.equal("No merge base found", err.message);
1319+
assert.equal(NodeGit.Error.CODE.ENOTFOUND, err.errno);
1320+
});
1321+
});
1322+
});
13011323
});

0 commit comments

Comments
 (0)