Fix the bugs in this HandleBookingOrder booking handler.
This HandleBookingOrder locks a user, books a flight with a retry loop, and returns a receipt. It does not compile and leaks the user lock on failure paths.
type BookingServiceError struct {
error
TryAgain bool
}
func (s *OrderService) HandleBookingOrder(user User) *Receipt {
receipt := Receipt{ID: uuid.New().String()}
if err := s.UserService.LockUser(user); err != nil {
log.Logger.Err(err)
return nil
}
// if err := s.UserService.UnlockUser(user); err != nil {
// return nil
// }
for {
bookingCode, err := s.BookingService.BookFlight()
if err != nil {
if err.TryAgain {
continue
}
log.Logger.Err(err)
return nil, err
}
receipt.BookedAt = time.Now().Format(time.RFC3339)
receipt.BookingCode = bookingCode
break
}
return &receipt
}Find and fix the bugs.
Change the signature to (*Receipt, error) and wrap each failure with %w. Move UnlockUser into a defer that captures and wraps its own error via a named return. Give BookingServiceError an Error() string, and in the loop use errors.As to recover it and check TryAgain instead of a bare field on the interface error.
- ✗Reading
err.TryAgaindirectly — the loop holdserras anerrorinterface, so the concrete field is unreachable withouterrors.As - ✗Calling
UnlockUserinline instead ofdefer, so an earlyreturnon a booking failure leaks the user lock - ✗Returning
nilon the lock-failure path of a(*Receipt, error)function, so the caller sees neither a receipt nor an error
- →Why must the deferred unlock use a named return parameter to surface its error?
- →How would you add a bounded retry count and backoff to the
TryAgainloop?
Find and fix the bugs
type BookingServiceError struct {
error
TryAgain bool
}
// must implement Error() string for BookingServiceError
func (s *OrderService) HandleBookingOrder(user User) *Receipt { // signature?
receipt := Receipt{ID: uuid.New().String()}
if err := s.UserService.LockUser(user); err != nil {
log.Logger.Err(err)
return nil // error is not returned
}
// unlock should move to a defer and be wrapped with context
// if err := s.UserService.UnlockUser(user); err != nil {
// return nil
// }
for {
bookingCode, err := s.BookingService.BookFlight()
if err != nil {
if err.TryAgain { // field read directly — needs errors.As/Is
continue
}
log.Logger.Err(err)
return nil, err // return type does not match the signature
}
receipt.BookedAt = time.Now().Format(time.RFC3339)
receipt.BookingCode = bookingCode
break
}
return &receipt
}Four defects: (1) the signature must be (*Receipt, error), and the return nil paths must return the error wrapped with %w; (2) UnlockUser must move into a defer with a named return so its error also surfaces; (3) BookingServiceError needs an Error() string method to be a valid error; (4) err.TryAgain is unreachable on the error interface — recover the concrete type with errors.As, then read TryAgain.
func (s *OrderService) HandleBookingOrder(user User) (_ *Receipt, err error) {
receipt := Receipt{ID: uuid.New().String()}
if e := s.UserService.LockUser(user); e != nil {
return nil, fmt.Errorf("lock user: %w", e)
}
defer func() {
if e := s.UserService.UnlockUser(user); e != nil && err == nil {
err = fmt.Errorf("unlock user: %w", e)
}
}()
for {
bookingCode, e := s.BookingService.BookFlight()
if e != nil {
var bookErr *BookingServiceError
if errors.As(e, &bookErr) && bookErr.TryAgain {
continue
}
return nil, fmt.Errorf("book flight: %w", e)
}
receipt.BookedAt = time.Now().Format(time.RFC3339)
receipt.BookingCode = bookingCode
break
}
return &receipt, nil
}
