Skip to content

Commit 40747aa

Browse files
nkhogennkhogen
committed
[PLAT-19116] Create Universe fails due to DeadlineExceeded being wrapped as INTERNAL
Summary: The original error was rethrown as INTERNAL. So, the change https://phorge.dev.yugabyte.com/D48608 was supposed to fix the error, but it caused it. Test Plan: UT to make sure it works. Tried a few times locally. Manually created a 4 node universe, stop/start processes ran fine. Reviewers: dshubin, amindrov Reviewed By: dshubin Subscribers: yugaware Differential Revision: https://phorge.dev.yugabyte.com/D48702
1 parent 7a5015e commit 40747aa

File tree

2 files changed

+55
-29
lines changed

2 files changed

+55
-29
lines changed

‎managed/node-agent/app/server/rpc.go‎

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,17 @@ func (server *RPCServer) Stop() {
204204
server.listener = nil
205205
}
206206

207+
func toGrpcErrorIfNeeded(code codes.Code, err error) error {
208+
if err == nil {
209+
return err
210+
}
211+
_, ok := status.FromError(err)
212+
if ok {
213+
return err
214+
}
215+
return status.Error(code, err.Error())
216+
}
217+
207218
/* Implementation of gRPC methods start here. */
208219

209220
// Ping handles ping request.
@@ -271,7 +282,7 @@ func (server *RPCServer) SubmitTask(
271282
shellTask := task.NewShellTaskWithUser("RemoteCommand", username, cmd[0], cmd[1:])
272283
err := task.GetTaskManager().Submit(ctx, taskID, shellTask)
273284
if err != nil {
274-
return res, status.Error(codes.Internal, err.Error())
285+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
275286
}
276287
return res, nil
277288
}
@@ -282,14 +293,14 @@ func (server *RPCServer) SubmitTask(
282293
err := util.ConvertType(preflightCheckInput, &preflightCheckParam)
283294
if err != nil {
284295
util.FileLogger().Errorf(ctx, "Error in preflight input conversion - %s", err.Error())
285-
return res, status.Error(codes.InvalidArgument, err.Error())
296+
return res, toGrpcErrorIfNeeded(codes.InvalidArgument, err)
286297
}
287298
preflightCheckHandler := task.NewPreflightCheckHandler(preflightCheckParam)
288299
err = task.GetTaskManager().
289300
Submit(ctx, taskID, preflightCheckHandler)
290301
if err != nil {
291302
util.FileLogger().Errorf(ctx, "Error in running preflight check - %s", err.Error())
292-
return res, status.Error(codes.Internal, err.Error())
303+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
293304
}
294305
return res, nil
295306
}
@@ -299,7 +310,7 @@ func (server *RPCServer) SubmitTask(
299310
err := task.GetTaskManager().Submit(ctx, taskID, downloadSoftwareHandler)
300311
if err != nil {
301312
util.FileLogger().Errorf(ctx, "Error in running download software - %s", err.Error())
302-
return res, status.Error(codes.Internal, err.Error())
313+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
303314
}
304315
res.TaskId = taskID
305316
return res, nil
@@ -310,7 +321,7 @@ func (server *RPCServer) SubmitTask(
310321
err := task.GetTaskManager().Submit(ctx, taskID, installSoftwareHandler)
311322
if err != nil {
312323
util.FileLogger().Errorf(ctx, "Error in running install software - %s", err.Error())
313-
return res, status.Error(codes.Internal, err.Error())
324+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
314325
}
315326
res.TaskId = taskID
316327
return res, nil
@@ -322,7 +333,7 @@ func (server *RPCServer) SubmitTask(
322333
err := task.GetTaskManager().Submit(ctx, taskID, serverControlHandler)
323334
if err != nil {
324335
util.FileLogger().Errorf(ctx, "Error in running server control - %s", err.Error())
325-
return res, status.Error(codes.Internal, err.Error())
336+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
326337
}
327338
res.TaskId = taskID
328339
return res, nil
@@ -339,7 +350,7 @@ func (server *RPCServer) SubmitTask(
339350
if err2 != nil {
340351
util.FileLogger().
341352
Errorf(ctx, "Error in running configure handler - %s", err2.Error())
342-
return res, status.Error(codes.Internal, err2.Error())
353+
return res, toGrpcErrorIfNeeded(codes.Internal, err2)
343354
}
344355
res.TaskId = taskID
345356
return res, nil
@@ -357,7 +368,7 @@ func (server *RPCServer) SubmitTask(
357368
err := task.GetTaskManager().Submit(ctx, taskID, serverGFlagsHandler)
358369
if err != nil {
359370
util.FileLogger().Errorf(ctx, "Error in running server gflags - %s", err.Error())
360-
return res, status.Error(codes.Internal, err.Error())
371+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
361372
}
362373
res.TaskId = taskID
363374
return res, nil
@@ -368,7 +379,7 @@ func (server *RPCServer) SubmitTask(
368379
err := task.GetTaskManager().Submit(ctx, taskID, installYbcHandler)
369380
if err != nil {
370381
util.FileLogger().Errorf(ctx, "Error in running install ybc - %s", err.Error())
371-
return res, status.Error(codes.Internal, err.Error())
382+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
372383
}
373384
res.TaskId = taskID
374385
return res, nil
@@ -379,7 +390,7 @@ func (server *RPCServer) SubmitTask(
379390
err := task.GetTaskManager().Submit(ctx, taskID, configureServerHandler)
380391
if err != nil {
381392
util.FileLogger().Errorf(ctx, "Error in running configure server - %s", err.Error())
382-
return res, status.Error(codes.Internal, err.Error())
393+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
383394
}
384395
res.TaskId = taskID
385396
return res, nil
@@ -394,7 +405,7 @@ func (server *RPCServer) SubmitTask(
394405
if err != nil {
395406
util.FileLogger().
396407
Errorf(ctx, "Error in running install otel collector - %s", err.Error())
397-
return res, status.Error(codes.Internal, err.Error())
408+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
398409
}
399410
res.TaskId = taskID
400411
return res, nil
@@ -409,7 +420,7 @@ func (server *RPCServer) SubmitTask(
409420
if err != nil {
410421
util.FileLogger().
411422
Errorf(ctx, "Error in running setup cGroup - %s", err.Error())
412-
return res, status.Error(codes.Internal, err.Error())
423+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
413424
}
414425
res.TaskId = taskID
415426
return res, nil
@@ -424,12 +435,12 @@ func (server *RPCServer) SubmitTask(
424435
if err != nil {
425436
util.FileLogger().
426437
Errorf(ctx, "Error in running destroy server - %s", err.Error())
427-
return res, status.Error(codes.Internal, err.Error())
438+
return res, toGrpcErrorIfNeeded(codes.Internal, err)
428439
}
429440
res.TaskId = taskID
430441
return res, nil
431442
}
432-
return res, status.Error(codes.Unimplemented, "Unknown task")
443+
return res, toGrpcErrorIfNeeded(codes.Unimplemented, errors.New("Unknown task"))
433444
}
434445

435446
// DescribeTask describes a submitted task.
@@ -475,7 +486,7 @@ func (server *RPCServer) DescribeTask(
475486
return nil
476487
})
477488
if err != nil && err != io.EOF {
478-
return status.Error(codes.Internal, err.Error())
489+
return toGrpcErrorIfNeeded(codes.Internal, err)
479490
}
480491
return nil
481492
}
@@ -487,7 +498,7 @@ func (server *RPCServer) AbortTask(
487498
) (*pb.AbortTaskResponse, error) {
488499
taskID, err := task.GetTaskManager().Abort(ctx, req.GetTaskId())
489500
if err != nil {
490-
return nil, status.Error(codes.Internal, err.Error())
501+
return nil, toGrpcErrorIfNeeded(codes.Internal, err)
491502
}
492503
return &pb.AbortTaskResponse{TaskId: taskID}, nil
493504
}
@@ -498,15 +509,15 @@ func (server *RPCServer) UploadFile(stream pb.NodeAgent_UploadFileServer) error
498509
req, err := stream.Recv()
499510
if err != nil {
500511
util.FileLogger().Errorf(ctx, "Error in receiving file info - %s", err.Error())
501-
return status.Error(codes.Internal, err.Error())
512+
return toGrpcErrorIfNeeded(codes.Internal, err)
502513
}
503514
fileInfo := req.GetFileInfo()
504515
filename := fileInfo.GetFilename()
505516
username := req.GetUser()
506517
chmod := req.GetChmod()
507518
userDetail, err := util.UserInfo(username)
508519
if err != nil {
509-
return status.Error(codes.Internal, err.Error())
520+
return toGrpcErrorIfNeeded(codes.Internal, err)
510521
}
511522
util.FileLogger().Debugf(ctx, "Using user: %s, uid: %d, gid: %d",
512523
userDetail.User.Username, userDetail.UserID, userDetail.GroupID)
@@ -524,22 +535,22 @@ func (server *RPCServer) UploadFile(stream pb.NodeAgent_UploadFileServer) error
524535
if err != nil {
525536
util.FileLogger().Errorf(
526537
ctx, "Error in deleting existing file %s - %s", filename, err.Error())
527-
return status.Error(codes.Internal, err.Error())
538+
return toGrpcErrorIfNeeded(codes.Internal, err)
528539
}
529540
util.FileLogger().Infof(ctx, "Setting file permission for %s to %o", filename, chmod)
530541
}
531542
file, err := os.OpenFile(filename, os.O_TRUNC|os.O_RDWR|os.O_CREATE, fs.FileMode(chmod))
532543
if err != nil {
533544
util.FileLogger().Errorf(ctx, "Error in creating file %s - %s", filename, err.Error())
534-
return status.Error(codes.Internal, err.Error())
545+
return toGrpcErrorIfNeeded(codes.Internal, err)
535546
}
536547
defer file.Close()
537548
if !userDetail.IsCurrent {
538549
err = file.Chown(int(userDetail.UserID), int(userDetail.GroupID))
539550
if err != nil {
540551
util.FileLogger().
541552
Errorf(ctx, "Error in changing file owner %s - %s", filename, err.Error())
542-
return status.Error(codes.Internal, err.Error())
553+
return toGrpcErrorIfNeeded(codes.Internal, err)
543554
}
544555
}
545556
// Flushes 4K bytes by default.
@@ -553,26 +564,26 @@ func (server *RPCServer) UploadFile(stream pb.NodeAgent_UploadFileServer) error
553564
}
554565
util.FileLogger().
555566
Errorf(ctx, "Error in flushing data to file %s - %s", filename, err.Error())
556-
return status.Error(codes.Internal, err.Error())
567+
return toGrpcErrorIfNeeded(codes.Internal, err)
557568
}
558569
if err != nil {
559570
util.FileLogger().Errorf(ctx, "Error in reading from stream - %s", err.Error())
560-
return status.Error(codes.Internal, err.Error())
571+
return toGrpcErrorIfNeeded(codes.Internal, err)
561572
}
562573
chunk := req.GetChunkData()
563574
size := len(chunk)
564575
util.FileLogger().Debugf(ctx, "Received a chunk with size: %d", size)
565576
_, err = writer.Write(chunk)
566577
if err != nil {
567578
util.FileLogger().Errorf(ctx, "Error in writing to file %s - %s", filename, err.Error())
568-
return status.Error(codes.Internal, err.Error())
579+
return toGrpcErrorIfNeeded(codes.Internal, err)
569580
}
570581
}
571582
res := &pb.UploadFileResponse{}
572583
err = stream.SendAndClose(res)
573584
if err != nil {
574585
util.FileLogger().Errorf(ctx, "Error in sending response - %s", err.Error())
575-
return status.Error(codes.Internal, err.Error())
586+
return toGrpcErrorIfNeeded(codes.Internal, err)
576587
}
577588
return nil
578589
}
@@ -589,7 +600,7 @@ func (server *RPCServer) DownloadFile(
589600
username := in.GetUser()
590601
userDetail, err := util.UserInfo(username)
591602
if err != nil {
592-
return status.Error(codes.Internal, err.Error())
603+
return toGrpcErrorIfNeeded(codes.Internal, err)
593604
}
594605
util.FileLogger().Debugf(ctx, "Using user: %s, uid: %d, gid: %d",
595606
userDetail.User.Username, userDetail.UserID, userDetail.GroupID)
@@ -598,7 +609,7 @@ func (server *RPCServer) DownloadFile(
598609
file, err := os.Open(filename)
599610
if err != nil {
600611
util.FileLogger().Errorf(ctx, "Error in opening file %s - %s", filename, err.Error())
601-
return status.Error(codes.Internal, err.Error())
612+
return toGrpcErrorIfNeeded(codes.Internal, err)
602613
}
603614
defer file.Close()
604615
// Reads 4K bytes by default.
@@ -610,13 +621,13 @@ func (server *RPCServer) DownloadFile(
610621
}
611622
if err != nil {
612623
util.FileLogger().Errorf(ctx, "Error in reading file %s - %s", filename, err.Error())
613-
return status.Error(codes.Internal, err.Error())
624+
return toGrpcErrorIfNeeded(codes.Internal, err)
614625
}
615626
res.ChunkData = res.ChunkData[:n]
616627
err = stream.Send(res)
617628
if err != nil {
618629
util.FileLogger().Errorf(ctx, "Error in sending file %s - %s", filename, err.Error())
619-
return status.Error(codes.Internal, err.Error())
630+
return toGrpcErrorIfNeeded(codes.Internal, err)
620631
}
621632
}
622633
return nil

‎managed/node-agent/app/server/rpc_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ import (
2323
"time"
2424

2525
"google.golang.org/grpc"
26+
"google.golang.org/grpc/codes"
2627
"google.golang.org/grpc/credentials/insecure"
28+
"google.golang.org/grpc/status"
2729
)
2830

2931
var (
@@ -513,3 +515,16 @@ func TestMetric(t *testing.T) {
513515
}
514516
t.Log(output)
515517
}
518+
519+
func TestErrorType(t *testing.T) {
520+
err := status.New(codes.DeadlineExceeded, "Client deadline exceeded").Err()
521+
st, ok := status.FromError(err)
522+
if ok {
523+
t.Logf("It is a grpc error. code: %v, message: %s", st.Code(), st.Message())
524+
if st.Code() != codes.DeadlineExceeded {
525+
t.Fatalf("Expected code %v, found %v", codes.DeadlineExceeded, st.Code())
526+
}
527+
} else {
528+
t.Fatalf("It is not a grpc error. error: %v", err)
529+
}
530+
}

0 commit comments

Comments
 (0)