Skip to content

Commit c9f159e

Browse files
mbroadstmbroadst
committed
fix(error): add proper context to error messages
NLVAsyncWorkerBase was previously modified to include context for error location when libvirt operations failed. This didn't translate over to the most recent async worker work, and therefore error context was consistently showing as being called from `nlv_async_worker.cc:58`, or the callsite of the error callback of the async worker. This patch corrects this behavior
1 parent 05040cd commit c9f159e

17 files changed

+168
-164
lines changed

‎src/domain.cc‎

Lines changed: 47 additions & 47 deletions
Large diffs are not rendered by default.

‎src/domain.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ class Domain : public NLVObject<Domain, virDomainPtr, DomainCleanupHandler>
473473
NLV_WORKER_ASSERT_DOMAIN();
474474
virErrorPtr error = func_(Handle());
475475
if(error) {
476-
SetVirError(error);
476+
SET_ERROR_WITH_CONTEXT(error);
477477
}
478478
}
479479
private:

‎src/error.cc‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -148,24 +148,22 @@ void Error::Initialize(Handle<Object> exports)
148148
NODE_DEFINE_CONSTANT(exports, VIR_ERR_ERROR);
149149
}
150150

151-
Error::Error(virErrorPtr error, const char* context)
152-
: Nan::ObjectWrap()
151+
Error::Error(virErrorPtr error, std::string context)
152+
: Nan::ObjectWrap(),
153+
error_(error),
154+
context_(context)
153155
{
154-
error_ = error;
155-
context_ = context;
156156
}
157157

158158
Error::~Error()
159159
{
160160
virFreeError(error_);
161161
}
162162

163-
Local<Value> Error::New(virErrorPtr error, const char* context)
163+
Local<Value> Error::New(virErrorPtr error, std::string context)
164164
{
165165
Nan::EscapableHandleScope scope;
166-
167166
Local<Function> ctor = Nan::New<Function>(constructor);
168-
169167
Local<Object> instance = Nan::NewInstance(ctor).ToLocalChecked();
170168
Error *err = new Error(error, context);
171169
err->Wrap(instance);
@@ -175,7 +173,6 @@ Local<Value> Error::New(virErrorPtr error, const char* context)
175173
NAN_GETTER(Error::Getter)
176174
{
177175
Nan::HandleScope scope;
178-
179176
Error *error = Nan::ObjectWrap::Unwrap<Error>(info.This());
180177
virErrorPtr error_ = error->error_;
181178

@@ -198,7 +195,7 @@ NAN_GETTER(Error::Getter)
198195
} else if (property->Equals(Nan::New("int2").ToLocalChecked())) {
199196
return info.GetReturnValue().Set(Nan::New(error_->int2));
200197
} else if (property->Equals(Nan::New("context").ToLocalChecked())) {
201-
return info.GetReturnValue().Set(Nan::New(error->context_).ToLocalChecked());
198+
return info.GetReturnValue().Set(Nan::New(error->context_.c_str()).ToLocalChecked());
202199
}
203200

204201
return;

‎src/error.h‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,26 @@ class Error : public Nan::ObjectWrap
1010
{
1111
public:
1212
static void Initialize(Handle<Object> exports);
13-
static Local<Value> New(virErrorPtr error, const char* context);
13+
static Local<Value> New(virErrorPtr error, std::string context);
1414

1515
private:
16-
explicit Error(virErrorPtr error, const char* context);
16+
explicit Error(virErrorPtr error, std::string context);
1717
~Error();
1818
static Nan::Persistent<Function> constructor;
1919

2020
static NAN_GETTER(Getter);
2121

2222
virErrorPtr error_;
23-
const char* context_;
23+
std::string context_;
2424
};
2525

2626
} //namespace NLV
2727

2828
#define _LIBVIRT_STRINGIZE_DETAIL(x) #x
2929
#define _LIBVIRT_STRINGIZE(x) _LIBVIRT_STRINGIZE_DETAIL(x)
30-
31-
#define ThrowLastVirError() Nan::ThrowError(Error::New(virSaveLastError(), __FILE__ ":" _LIBVIRT_STRINGIZE(__LINE__)))
32-
#define MakeVirError() Error::New(VirError(), __FILE__ ":" _LIBVIRT_STRINGIZE(__LINE__))
30+
#define ThrowLastVirError() \
31+
Nan::ThrowError(Error::New(virSaveLastError(), __FILE__ ":" _LIBVIRT_STRINGIZE(__LINE__)))
32+
#define SET_ERROR_WITH_CONTEXT(error) \
33+
SetVirError(error, __FILE__ ":" _LIBVIRT_STRINGIZE(__LINE__))
3334

3435
#endif // ERROR_H

‎src/hypervisor.cc‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ NLV_WORKER_EXECUTE(Hypervisor, Disconnect)
296296
hypervisor_->ClearHandle();
297297
// int result = virConnectClose(Handle());
298298
// if (result == -1) {
299-
// SetVirError(virSaveLastError());
299+
// SET_ERROR_WITH_CONTEXT(virSaveLastError());
300300
// return;
301301
// }
302302

@@ -309,7 +309,7 @@ NLV_WORKER_EXECUTE(Hypervisor, Disconnect)
309309
NLV_WORKER_ASSERT_CONNECTION(); \
310310
char *result = Accessor(Handle()); \
311311
if (result == NULL) { \
312-
SetVirError(virSaveLastError()); \
312+
SET_ERROR_WITH_CONTEXT(virSaveLastError()); \
313313
return; \
314314
} \
315315
data_ = result; \
@@ -321,7 +321,7 @@ NLV_WORKER_EXECUTE(Hypervisor, Disconnect)
321321
NLV_WORKER_ASSERT_CONNECTION(); \
322322
const char *result = Accessor(Handle()); \
323323
if (result == NULL) { \
324-
SetVirError(virSaveLastError()); \
324+
SET_ERROR_WITH_CONTEXT(virSaveLastError()); \
325325
return; \
326326
} \
327327
data_ = result; \
@@ -340,7 +340,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetSysInfo)
340340
NLV_WORKER_ASSERT_CONNECTION();
341341
char *result = virConnectGetSysinfo(Handle(), 0);
342342
if (result == NULL) {
343-
SetVirError(virSaveLastError());
343+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
344344
return;
345345
}
346346

@@ -362,7 +362,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetVersion)
362362
unsigned long version;
363363
int result = virConnectGetVersion(Handle(), &version);
364364
if (result == -1) {
365-
SetVirError(virSaveLastError());
365+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
366366
return;
367367
} else if (result == 0 && version == 0) {
368368
return;
@@ -386,7 +386,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetLibVirtVersion)
386386
unsigned long version;
387387
int result = virConnectGetLibVersion(Handle(), &version);
388388
if (result == -1) {
389-
SetVirError(virSaveLastError());
389+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
390390
return;
391391
}
392392

@@ -404,7 +404,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetLibVirtVersion)
404404
NLV_WORKER_ASSERT_CONNECTION(); \
405405
int result = Accessor(Handle()); \
406406
if (result == -1) { \
407-
SetVirError(virSaveLastError()); \
407+
SET_ERROR_WITH_CONTEXT(virSaveLastError()); \
408408
return; \
409409
} \
410410
data_ = result; \
@@ -450,7 +450,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetMaxVcpus)
450450
NLV_WORKER_ASSERT_CONNECTION();
451451
int result = virConnectGetMaxVcpus(Handle(), type_.c_str());
452452
if (result == -1) {
453-
SetVirError(virSaveLastError());
453+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
454454
return;
455455
}
456456

@@ -496,7 +496,7 @@ NLV_WORKER_EXECUTE(Hypervisor, SetKeepAlive)
496496

497497
int result = virConnectSetKeepAlive(Handle(), interval_, count_);
498498
if (result == -1) {
499-
SetVirError(virSaveLastError());
499+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
500500
return;
501501
}
502502

@@ -540,7 +540,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetBaselineCPU)
540540
delete [] cpus_;
541541

542542
if (result == NULL) {
543-
SetVirError(virSaveLastError());
543+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
544544
return;
545545
}
546546

@@ -570,7 +570,7 @@ NLV_WORKER_EXECUTE(Hypervisor, CompareCPU)
570570
NLV_WORKER_ASSERT_CONNECTION();
571571
int result = virConnectCompareCPU(Handle(), (const char *)cpu_.c_str(), flags_);
572572
if (result == VIR_CPU_COMPARE_ERROR) {
573-
SetVirError(virSaveLastError());
573+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
574574
return;
575575
}
576576

@@ -582,7 +582,7 @@ NLV_WORKER_EXECUTE(Hypervisor, CompareCPU)
582582
NLV_WORKER_ASSERT_CONNECTION() \
583583
int count = CountMethod(Handle()); \
584584
if (count == -1) { \
585-
SetVirError(virSaveLastError()); \
585+
SET_ERROR_WITH_CONTEXT(virSaveLastError()); \
586586
return; \
587587
} \
588588
char **names = new char*[count]; \
@@ -592,7 +592,7 @@ NLV_WORKER_EXECUTE(Hypervisor, CompareCPU)
592592
} \
593593
int nameCount = ListMethod(Handle(), names, count); \
594594
if (nameCount == -1) { \
595-
SetVirError(virSaveLastError()); \
595+
SET_ERROR_WITH_CONTEXT(virSaveLastError()); \
596596
delete [] names; \
597597
return; \
598598
} \
@@ -608,7 +608,7 @@ NLV_WORKER_EXECUTE(Hypervisor, CompareCPU)
608608
NLV_WORKER_ASSERT_CONNECTION() \
609609
int count = CountMethod(Handle()); \
610610
if (count == -1) { \
611-
SetVirError(virSaveLastError()); \
611+
SET_ERROR_WITH_CONTEXT(virSaveLastError()); \
612612
return; \
613613
} \
614614
int *elements = new int[count]; \
@@ -619,7 +619,7 @@ NLV_WORKER_EXECUTE(Hypervisor, CompareCPU)
619619
} \
620620
int elementCount = ListMethod(Handle(), elements, count); \
621621
if (elementCount == -1) { \
622-
SetVirError(virSaveLastError()); \
622+
SET_ERROR_WITH_CONTEXT(virSaveLastError()); \
623623
return; \
624624
} \
625625
for (int i = 0; i < elementCount; ++i) { \
@@ -730,7 +730,7 @@ NLV_WORKER_EXECUTE(Hypervisor, ListNodeDevices)
730730
virNodeNumOfDevices(Handle(), (const char *) capability_.c_str(), flags);
731731

732732
if (num_devices == -1) {
733-
SetVirError(virSaveLastError());
733+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
734734
return;
735735
}
736736

@@ -744,7 +744,7 @@ NLV_WORKER_EXECUTE(Hypervisor, ListNodeDevices)
744744
virNodeListDevices(Handle(), (const char *)capability_.c_str(), names, num_devices, flags);
745745
if (num_devices == -1) {
746746
free(names);
747-
SetVirError(virSaveLastError());
747+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
748748
return;
749749
}
750750

@@ -762,7 +762,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetNodeSecurityModel)
762762
NLV_WORKER_ASSERT_CONNECTION();
763763
int result = virNodeGetSecurityModel(Handle(), &securityModel_);
764764
if (result == -1) {
765-
SetVirError(virSaveLastError());
765+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
766766
return;
767767
}
768768
}
@@ -789,7 +789,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetNodeInfo)
789789
NLV_WORKER_ASSERT_CONNECTION();
790790
int result = virNodeGetInfo(Handle(), &info_);
791791
if (result == -1) {
792-
SetVirError(virSaveLastError());
792+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
793793
return;
794794
}
795795
}
@@ -818,7 +818,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetNodeFreeMemory)
818818
NLV_WORKER_ASSERT_CONNECTION();
819819
unsigned long long result = virNodeGetFreeMemory(Handle());
820820
if (result == 0) {
821-
SetVirError(virSaveLastError());
821+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
822822
return;
823823
}
824824

@@ -851,7 +851,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetNodeMemoryStats)
851851
result = virNodeGetMemoryStats(Handle(), cellNum_, &info_[0], &nparams, flags_);
852852
}
853853
if (result == -1) {
854-
SetVirError(virSaveLastError());
854+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
855855
return;
856856
}
857857
}
@@ -904,7 +904,7 @@ NLV_WORKER_EXECUTE(Hypervisor, GetNodeCellsFreeMemory)
904904
virNodeGetCellsFreeMemory(Handle(), results, startCell_, maxCells_);
905905
if (cells == -1) {
906906
free(results);
907-
SetVirError(virSaveLastError());
907+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
908908
return;
909909
}
910910

‎src/interface.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ NLV_WORKER_EXECUTE(Interface, Start)
4040
unsigned int flags = 0;
4141
int result = virInterfaceCreate(Handle(), flags);
4242
if (result == -1) {
43-
SetVirError(virSaveLastError());
43+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
4444
return;
4545
}
4646

@@ -55,7 +55,7 @@ NLV_WORKER_EXECUTE(Interface, Stop)
5555
unsigned int flags = 0;
5656
int result = virInterfaceDestroy(Handle(), flags);
5757
if (result == -1) {
58-
SetVirError(virSaveLastError());
58+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
5959
return;
6060
}
6161

@@ -68,7 +68,7 @@ NLV_WORKER_EXECUTE(Interface, Define)
6868
unsigned int flags = 0;
6969
lookupHandle_ = virInterfaceDefineXML(parent_->virHandle(), value_.c_str(), flags);
7070
if (lookupHandle_ == NULL) {
71-
SetVirError(virSaveLastError());
71+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
7272
return;
7373
}
7474
}
@@ -79,7 +79,7 @@ NLV_WORKER_EXECUTE(Interface, Undefine)
7979
NLV_WORKER_ASSERT_INTERFACE();
8080
int result = virInterfaceUndefine(Handle());
8181
if (result == -1) {
82-
SetVirError(virSaveLastError());
82+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
8383
return;
8484
}
8585

@@ -138,7 +138,7 @@ NLV_WORKER_EXECUTE(Interface, GetName)
138138
NLV_WORKER_ASSERT_INTERFACE();
139139
const char *result = virInterfaceGetName(Handle());
140140
if (result == NULL) {
141-
SetVirError(virSaveLastError());
141+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
142142
return;
143143
}
144144

@@ -151,7 +151,7 @@ NLV_WORKER_EXECUTE(Interface, GetMacAddress)
151151
NLV_WORKER_ASSERT_INTERFACE();
152152
const char *result = virInterfaceGetMACString(Handle());
153153
if (result == NULL) {
154-
SetVirError(virSaveLastError());
154+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
155155
return;
156156
}
157157

@@ -164,7 +164,7 @@ NLV_WORKER_EXECUTE(Interface, IsActive)
164164
NLV_WORKER_ASSERT_INTERFACE();
165165
int result = virInterfaceIsActive(Handle());
166166
if (result == -1) {
167-
SetVirError(virSaveLastError());
167+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
168168
return;
169169
}
170170

@@ -178,7 +178,7 @@ NLV_WORKER_EXECUTE(Interface, ToXml)
178178
unsigned int flags = 0;
179179
char *result = virInterfaceGetXMLDesc(Handle(), flags);
180180
if (result == NULL) {
181-
SetVirError(virSaveLastError());
181+
SET_ERROR_WITH_CONTEXT(virSaveLastError());
182182
return;
183183
}
184184

0 commit comments

Comments
 (0)