Pull to refresh

Топ-10 самых распространенных ошибок, которые мне встречались в Go-проектах

Reading time10 min
Views8.5K
Original author: Teiva Harsanyi
Этот пост — мой топ самых распространенных ошибок, которые встречались мне в Go-проектах. Порядок не имеет значения.

image

Неизвестное значение Enum


Давайте взглянем на простой пример:

type Status uint32

const (
	StatusOpen Status = iota
	StatusClosed
	StatusUnknown
)

Здесь мы создаем энумератор с помощью iota, что приведет к такому состоянию:

StatusOpen = 0
StatusClosed = 1
StatusUnknown = 2

Теперь давайте представим, что этот тип Status является частью JSON запроса, который будет упаковываться/распаковываться. Мы можем спроектировать следующую структуру:

type Request struct {
	ID        int    `json:"Id"`
	Timestamp int    `json:"Timestamp"`
	Status    Status `json:"Status"`
}

Затем получаем такой результат запроса:

{
  "Id": 1234,
  "Timestamp": 1563362390,
  "Status": 0
}

В целом ничего особенного, — Status будет распакован в StatusOpen.
Теперь, давайте получим еще один ответ, в котором значение статуса не установлено:

{
  "Id": 1235,
  "Timestamp": 1563362390
}

В этом случае поле Status структуры Request будет инициализировано нулевым значением (для uint32 – это 0). Следовательно, мы опять получим StatusOpen вместо StatusUnknown.

В этом случае лучше всего ставить неизвестное значение энумератора первым – т.е. 0:

type Status uint32

const (
	StatusUnknown Status = iota
	StatusOpen
	StatusClosed
)

Если статус не является частью запроса JSON, он будет инициализирован в StatusUnknown, как и мы и ожидаем.

Бенчмаркинг


Правильный бенчмаркинг — это довольно сложно. Слишком много факторов могут повлиять на результат.

Одна из распространенных ошибок – быть обманутым оптимизациями компилятора. Давайте посмотрим конкретный пример из библиотеки teivah/bitvector:

func clear(n uint64, i, j uint8) uint64 {
	return (math.MaxUint64<<j | ((1 << i) - 1)) & n
}

Эта функция очищает биты в заданном диапазоне. Затестить производительность мы можем таким образом:

func BenchmarkWrong(b *testing.B) {
	for i := 0; i < b.N; i++ {
		clear(1221892080809121, 10, 63)
	}
}

В этом тесте компилятор заметит, что clear не вызывает никакой другой функции, поэтому просто встроит ее как есть. Как только она будет встроена, компилятор увидит, что никаких сайд-эффектов не возникает. Таким образом, вызов clear будет просто удален, что приведет к неточным результатам.

Одним из решений может быть установка результата в глобальную переменную, типа такого:

var result uint64

func BenchmarkCorrect(b *testing.B) {
	var r uint64
	for i := 0; i < b.N; i++ {
		r = clear(1221892080809121, 10, 63)
	}
	result = r
}

Тут компилятор не будет знать, создает ли вызов сайд-эффект. Поэтому бенчмарк будет точным.

Указатели! Указатели везде!


Передача переменной по значению создаст копию этой переменной. В то время как передача по указателю просто скопирует адрес в памяти.

Следовательно, передача указателя всегда будет быстрее, так?

Если вы так считаете, взгляните на данный пример. Это бенчмарк для структуры данных объемом 0,3 КБ, которую мы сперва передаем и получаем по указателю, а затем по значению. 0,3 КБ – это немного – примерно столько занимают обычные структуры данных, с которыми мы ежедневно работаем.

Когда я запускаю эти тесты в локальной среде, передача по значению выходит быстрее более чем в 4 раза. Довольно неожиданно, правда?

Объяснение такого результата связано с понимание, как происходит управление памятью в Go. Я не смогу объяснить это так же блестяще, как Уильям Кеннеди, но давайте попробуем обобщить в двух словах.

Переменная может быть размещена в куче или стеке:
  • Стек содержит текущие переменные данной программы. Как только функция делает return, переменные извлекаются из стека.
  • Куча содержит общие переменные (глобальные переменные и т. д.).

Давайте рассмотрим простой пример, где мы возвращаем значение:

func getFooValue() foo {
	var result foo
	// Do something
	return result
}

Здесь переменная result создается текущей горутиной. Эта переменная помещается в текущий стек. Как только функция вернет значение, клиент получит копию этой переменной. Сама переменная извлекается из стека. Она еще существует в памяти, пока не будет перезаписана другой переменной, но к ней уже нельзя получить доступ.
Теперь тот же пример, но уже с указателем:

func getFooPointer() *foo {
	var result foo
	// Do something
	return &result
}

Переменная result все еще создается текущей горутиной, но клиент получит указатель (копию адреса переменной). Если переменная result была извлечена из стека, клиент этой функции не сможет получить к ней доступ.

В этом сценарии компилятор Go будет выводить переменную result туда, где переменные могут использоваться совместно, т.е. в кучу.

Другой сценарий передачи указателей:

func main()  {
	p := &foo{}
	f(p)
}

Поскольку мы вызываем f в той же программе, переменную p не нужно выводить в кучу. Она просто помещается в стек, и подфункция может получить к ней доступ.

Например, таким образом происходит получения слайса (slice) в методе Read из io.Reader. Возврат слайса (который является указателем) выводит его в кучу.

Почему стек так быстро работает? Тут две причины:
  • Не нужно использовать сборщик мусора в стеке. Как мы уже говорили, переменная просто пушится (push) после создания, а затем извлекается (pop) из стека, когда функция делает return. Не нужно мутить сложный процесс, чтобы вернуть неиспользуемые переменные и т. д.
  • Стек принадлежит одной горутине, поэтому хранение переменной не нужно синхронизировать, как это происходит с хранением в куче, что также приводит к увеличению производительности.

В заключение, когда мы создаем функцию, нашим дефолтным (default) действием должно быть использование значений вместо указателей. Указатель должен использоваться, только если мы хотим расшаривать (share) переменную.

Так же, если мы страдаем от проблем производительности, одной из возможных оптимизаций может быть проверка, помогут ли указатели в конкретных ситуациях? Выводит ли компилятор переменную в кучу можно узнать с помощью следующей команды:
go build -gcflags "-m -m"
.
Но, опять же, для большинства наших ежедневных задач использование значений подходит лучше всего.

Прерывание for/switch или for/select


Что происходит в следующем примере, если f возвращает true?

for {
  switch f() {
  case true:
    break
  case false:
    // Do something
  }
}

Мы вызываем break. Только данный break прерывает switch, а не цикл for.

Та же проблема здесь:

for {
  select {
  case <-ch:
  // Do something
  case <-ctx.Done():
    break
  }
}

Break связан с оператором select, а не с циклом for.

Одним из возможных решений для прерывания for/switch или for/select является использование метки:

loop:
	for {
		select {
		case <-ch:
		// Do something
		case <-ctx.Done():
			break loop
		}
	}

Обработка ошибок


Go все еще молод, особенно в сфере обработки ошибок. Преодоление данного недостатка — одно из самых ожидаемых нововведений в Go 2.

Текущая стандартная библиотека (до Go 1.13) предлагает только функции для конструирования ошибок. Поэтому будет любопытно взглянуть на пакет pkg/errors.

Данная библиотека — неплохой способ соблюдать правило, которое не всегда соблюдается:
Ошибка должна быть обработана только один раз. Логирование ошибки — это обработка ошибки
. Таким образом, ошибка должна логироваться или прокидываться выше.

В текущей стандартной библиотеке этот принцип соблюсти трудно, так как мы можем захотеть добавить контекст к ошибке и иметь какую-то иерархию.

Давайте рассмотрим пример с вызовом REST, приводящим к ошибке БД:

unable to server HTTP POST request for customer 1234
 |_ unable to insert customer contract abcd
     |_ unable to commit transaction

Если мы воспользуемся pkg/errors, то можно поступить следующим образом:

func postHandler(customer Customer) Status {
	err := insert(customer.Contract)
	if err != nil {
		log.WithError(err).Errorf("unable to server HTTP POST request for customer %s", customer.ID)
		return Status{ok: false}
	}
	return Status{ok: true}
}

func insert(contract Contract) error {
	err := dbQuery(contract)
	if err != nil {
		return errors.Wrapf(err, "unable to insert customer contract %s", contract.ID)
	}
	return nil
}

func dbQuery(contract Contract) error {
	// Do something then fail
	return errors.New("unable to commit transaction")
}

Начальная ошибка (если она не возвращена внешней библиотекой) может быть создана с помощью errors.New. Средний слой, — insert, — оборачивает эту ошибку, добавляя к ней больше контекста. Затем родитель ее логирует. Таким образом, каждый уровень либо возвращает, либо обрабатывает ошибку.

Мы также можем захотеть найти причину ошибки, например, для повторного вызова. Допустим, у нас есть пакет db из внешней библиотеки, который имеет доступ к базе данных. Эта библиотека может возвращать временную ошибку с именем db.DBError. Чтобы определить, нужно ли нам повторить попытку, мы должны установить причину ошибки:

func postHandler(customer Customer) Status {
	err := insert(customer.Contract)
	if err != nil {
		switch errors.Cause(err).(type) {
		default:
			log.WithError(err).Errorf("unable to server HTTP POST request for customer %s", customer.ID)
			return Status{ok: false}
		case *db.DBError:
			return retry(customer)
		}

	}
	return Status{ok: true}
}

func insert(contract Contract) error {
	err := db.dbQuery(contract)
	if err != nil {
		return errors.Wrapf(err, "unable to insert customer contract %s", contract.ID)
	}
	return nil
}

Это делается с помощью errors.Cause, которая также входит в pkg/errors:

Одной из частых ошибок, которые мне встречались, было использование pkg/errors лишь частично. Проверка ошибки, например, была выполнена следующим образом:

switch err.(type) {
default:
  log.WithError(err).Errorf("unable to server HTTP POST request for customer %s", customer.ID)
  return Status{ok: false}
case *db.DBError:
  return retry(customer)
}

В этом примере, если db.DBError обернут (wrapped), он никогда не сделает повторный вызов.

Инициализация слайса


Иногда мы знаем, какой будет конечная длина слайса. Например, допустим, мы хотим преобразовать слайс Foo в слайс Bar, что означает, что эти два слайса будут иметь одинаковую длину.

Я часто встречаю слайсы, инициализированные подобным образом:

var bars []Bar
bars := make([]Bar, 0)

Слайс не является магической структурой. Под капотом он реализует стратегию увеличения размера, если свободного места больше нет. В этом случае автоматически создается новый массив (с большей емкостью), и все элементы копируются в него.

Теперь давайте представим, что нам нужно повторить эту операцию увеличения размера несколько раз, так как наш []Foo содержит тысячи элементов. Сложность алгоритма вставки останется O(1), но на практике это повлияет на производительность.

Поэтому, если мы знаем окончательную длину, мы можем либо:

  • Инициализировать его с заранее определенной длиной:

func convert(foos []Foo) []Bar {
	bars := make([]Bar, len(foos))
	for i, foo := range foos {
		bars[i] = fooToBar(foo)
	}
	return bars
}

  • Либо инициализировать его с длиной 0 и предопределенным capacity:

func convert(foos []Foo) []Bar {
	bars := make([]Bar, 0, len(foos))
	for _, foo := range foos {
		bars = append(bars, fooToBar(foo))
	}
	return bars
}

Какой самый лучший вариант? Первый чуть быстрее. Тем не менее, вы можете предпочесть второй, потому что он более согласован: независимо от того, знаем ли мы начальный размер, добавление элемента в конце слайса выполняется с использованием append.

Управление контекстом


context.Context довольно часто неправильно понимается разработчиками. Согласно официальной документации:
Контекст переносит deadline, сигнал отмены и другие значения сквозь границы API.
Это описание достаточно общее, поэтому способно озадачить программиста как следует его правильно использовать.

Давайте попробуем разобраться. Контекст может переносить:
  • Дедлайн — означает либо длительность (например, 250 мс), либо дату-время (например, 2019-01-08 01:00:00), по которой мы считаем, что если она достигнута, нужно отменить текущее действие (запрос ввода-вывода), ожидание ввода канала и т. д.).
  • Сигнал отмены (в основном <-chan struct {}). Здесь поведение похожее. Как только мы получим сигнал, мы должны остановить текущую работу. Например, давайте представим, что мы получаем два запроса. Один для вставки данных, а другой для отмены первого запроса (потому что он больше не актуален, например). Этого можно достичь, используя отменяемый контекст в первом вызове, который затем будет отменен, как только мы получим второй запрос.
  • Список ключ/значение (оба основаны на типе interface{}).

Еще два тезиса. Во-первых, контекст является составным (composable). Поэтому, у нас может быть контекст, который переносит дедлайн и список ключ/значение, например. Кроме того, несколько горутин могут совместно использовать один и тот же контекст, поэтому сигнал отмены может потенциально остановить несколько работ.

Возвращаясь к нашей теме, вот ошибка, которая мне встретилась.

Приложение Go было основано на urfave/cli (если не знаете, это хорошая библиотека для создания приложений командной строки в Go). После запуска разработчик наследует своего рода контекст приложения. Это означает, что когда приложение остановлено, библиотека будет использовать контекст для отправки сигнала отмены.

Я заметил, что данный контекст передавался напрямую, например, при вызове gRPC-эндпоинта. Это совсем не то, что нам нужно.

Вместо этого мы хотим сказать библиотеке gRPC: пожалуйста, отмени запрос, когда приложение остановлено, или через 100 мс, например.

Чтобы достичь этого, мы можем просто создать составной контекст. Если parent — это имя контекста приложения (созданного urfave/cli), тогда мы можем просто сделать так:

ctx, cancel := context.WithTimeout(parent, 100 * time.Millisecond)
response, err := grpcClient.Send(ctx, request)

Контексты не столь сложны для понимания, и, на мой взгляд, это одна из лучших особенностей языка.

Не использование опции -race


Тестирование приложения Go без опции -race – это ошибка, которую я постоянно встречаю.

Как написано в этой статье, хотя Go был «разработан для того, чтобы сделать параллельное программирование более простым и менее подверженным ошибкам», мы все еще сильно страдаем от проблем параллелизма.

Очевидно, что race детектор Go не поможет при любой проблеме. Тем не менее, это ценный инструмент, и мы всегда должны включать его при тестировании наших приложений.

Использование имени файла в качестве инпута


Другая распространенная ошибка — передавать имя файла в функцию.

Допустим, нам нужно реализовать функцию для подсчета количества пустых строк в файле. Наиболее естественная реализация будет выглядеть примерно так:

func count(filename string) (int, error) {
	file, err := os.Open(filename)
	if err != nil {
		return 0, errors.Wrapf(err, "unable to open %s", filename)
	}
	defer file.Close()

	scanner := bufio.NewScanner(file)
	count := 0
	for scanner.Scan() {
		if scanner.Text() == "" {
			count++
		}
	}
	return count, nil
}

Имя файла задается в качестве инпута, поэтому мы открываем его и затем реализуем нашу логику, так?

Теперь предположим, что мы хотим покрыть эту функцию юнит-тестами. Мы будем тестировать с обычным файлом, пустым файлом, файлом с другим типом кодировки и т. Д. Управлять им может оказаться очень сложно.

Кроме того, если мы хотим реализовать ту же логику, например, для HTTP body, нам нужно будет создать для этого еще одну функцию.

Go поставляется с двумя отличными абстракциями: io.Reader и io.Writer. Вместо передачи имени файла мы можем просто передать io.Reader, который будет абстрагировать источник данных.
Это файл? HTTP body? Байтовый буфер? Не важно, так как мы все равно будем использовать один и тот же метод Read.

В нашем случае мы можем даже буферизовать инпут, чтобы читать его построчно. Для этого можно использовать bufio.Reader и его метод ReadLine:

func count(reader *bufio.Reader) (int, error) {
	count := 0
	for {
		line, _, err := reader.ReadLine()
		if err != nil {
			switch err {
			default:
				return 0, errors.Wrapf(err, "unable to read")
			case io.EOF:
				return count, nil
			}
		}
		if len(line) == 0 {
			count++
		}
	}
}

Теперь ответственность за открытие файла делегирована клиенту count:

file, err := os.Open(filename)
if err != nil {
  return errors.Wrapf(err, "unable to open %s", filename)
}
defer file.Close()
count, err := count(bufio.NewReader(file))

Во второй реализации функция может быть вызвана независимо от фактического источника данных. Между тем, и это облегчит наше юнит-тестирование, так как мы можем просто создать bufio.Reader из строки:

count, err := count(bufio.NewReader(strings.NewReader("input")))

Горутины и переменные цикла


Последняя распространенная ошибка, которую я встречал, возникала при использовании горутин с переменными цикла.

Каким будет вывод следующего примера?

ints := []int{1, 2, 3}
for _, i := range ints {
  go func() {
    fmt.Printf("%v\n", i)
  }()
}

1 2 3 в случайном порядке? Неа.

В этом примере каждая горутина использует один и тот же экземпляр переменной, поэтому она будет выводить 3 3 3 (скорее всего).

Есть два решения данной проблемы. Первое — передать значение переменной i в замыкание (внутренняя функция):

ints := []int{1, 2, 3}
for _, i := range ints {
  go func(i int) {
    fmt.Printf("%v\n", i)
  }(i)
}

Второе — создать еще одну переменную в пределах цикла for:

ints := []int{1, 2, 3}
for _, i := range ints {
  i := i
  go func() {
    fmt.Printf("%v\n", i)
  }()
}

Присваивать i := i может показаться немного странным, но эта конструкция совершенно валидна. Находиться в цикле означает находиться в другой области видимости. Поэтому i := i создает еще один экземпляр переменной i. Конечно, мы можем назвать его с другим именем для удобства чтения.

Если вы знаете другие распространенные ошибки, не стесняйтесь написать о них в комментариях.
Tags:
Hubs:
Total votes 26: ↑23 and ↓3+20
Comments7

Articles