diff --git a/CHANGELOG.md b/CHANGELOG.md index 1285827..98f8fe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,63 @@ ### Added +- **REST API 3.0 — по адресу вызова.** Чтобы вызывать методы новой версии, + достаточно передать адрес с сегментом `/rest/api/`. Опции для этого нет и не + будет: + + ```go + client := b24.NewClient("https://portal.bitrix24.ru/rest/api/1/токен/") + res, err := client.Core().Call(ctx, "tasks.task.list", b24.Params{ + "select": []string{"id", "title"}, + "pagination": b24.Params{"limit": 20, "page": 1}, + }) + ``` + + Версия выводится из адреса, потому что адрес её и так задаёт: без `/api/` + портал выполнит метод старой версии или ответит «метод не найден». Второй + источник истины можно рассогласовать с первым — сказать «версия 3» и забыть + `/api/`, — и тогда каждый вызов уходил бы в v1, а разбирался бы по правилам v3. + + Мешало ровно одно: SDK безусловно дописывал к методу `.json`, а v3 такой суффикс + не принимает — `…/rest/api/1/токен/documentation` отвечает 200, а + `documentation.json` — 404 «Метод `documentation.json` не найден». Теперь суффикс + дописывается только для v1, и **поведение v1 не изменилось ни в чём** — на это + есть отдельные регрессионные тесты. + + Конверт **успешного** ответа у v3 тот же, что у v1, поэтому `CallResult`, + `Result`, `Kind`, `Unwrap`, `IsEmpty`, `ID` работают без изменений. + +- **Разбор ошибок REST 3.0.** У v3 код и текст лежат во вложенном объекте + (`{"error":{"code":…,"message":…}}`), а не плоско, — `*APIError` заполняется + из обеих форм, и `errors.Is`, `CodeOf` и вся таксономия продолжают работать. + + Форму выбирает **тип поля `error`, а не версия адреса**: адрес v3 отвечает + обеими. Замер: `…/rest/api/…/tasks.task.get` с `{"id":"abc"}` отвечает HTTP 500 + и **плоским** телом `{"error":"INTERNAL_SERVER_ERROR",…}` — это докладывает + шлюз REST, стоящий перед контроллером v3. Разбор по версии потерял бы код всех + таких ошибок, включая `QUERY_LIMIT_EXCEEDED`, на котором держатся повторы. + + Коды у версий разные, и SDK сводит к старому **один** — тот, у которого на + обеих версиях одно и то же значение: `errors.Is(err, ErrMethodNotFound)` + срабатывает и на `BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION`. Остальные + не сводятся намеренно: `…_ACCESSDENIEDEXCEPTION` похож на `ACCESS_DENIED`, но + v3 отвечает им и на неверный токен вебхука, где v1 отвечает + `INVALID_CREDENTIALS`, — один код v3 покрывает два кода v1, и сведение + заставило бы ветку «права не те, авторизация в порядке» срабатывать на + протухшей авторизации. Для них — `ErrV3Validation`, `ErrV3EntityNotFound`, + `ErrV3AccessDenied` и константы `CodeV3*`. + + Приставка `BITRIX_REST_V3_EXCEPTION_` **не универсальна**, выводить код из неё + нельзя: `crm.deal.timeline.activity.email.list` на плохой `id` отвечает + `CRM_EMAIL_INVALID_REQUEST` в том же конверте. Поэтому `CodeOf` возвращает код + **как он пришёл**, без перевода: он идёт в лог, и чужой код там отправил бы + читателя искать строку, которой портал не присылал. + +- **`APIError.Validation`** — поля, из-за которых v3 отклонил запрос. Код и текст + у всех таких ошибок одинаково общие («Ошибка при валидации объекта запроса»), + так что список полей — единственная часть, говорящая, что именно не так. + У v1 аналога нет, там он пустой. + - **`Result` с `Kind()`.** Одно и то же поле Битрикс24 отвечает разной формой, и какой именно — зависит от **данных**, а не от метода: свойство товара с одним значением приходит объектом, с несколькими — массивом тех же объектов; а @@ -158,6 +215,61 @@ из `crm.deal.update` и `crm.deal.get` — два нуля и ошибка, называющая обе команды и цитирующая их ответы. Всё созданное удалено этими же идентификаторами, отсутствие проверено чтением. +### Changed + +- **`Pages`/`Scan` и `CallBatch`/`CallBatchChunked` на адресе v3 отказываются + работать** — `ErrV3WalkUnsupported` и `ErrV3BatchUnsupported`, до отправки + запроса. + + У v3 нет курсора: `start` игнорируется, `next` и `total` в ответе + отсутствуют, страница задаётся параметром `pagination` (`page`, `limit`, + `offset`). Без отказа обход выглядел бы работающим: на живом портале `Pages` + по `tasks.task.list` прочитал первую страницу, не увидел `next` и отчитался о + **завершённом** обходе с `Err() == nil` — 2 строки из 423. Частичная выгрузка, + выглядящая как полная, — то, чего у обхода быть не должно. + + Метод `batch` у v3 есть, но это другой протокол: команды кладутся в **корень** + тела как `{"method": …, "query": {…}}`, ответ — **массив** в порядке отправки + (ключи команд отбрасываются), а первая упавшая команда обрывает весь запрос + вместо `result_error`. Поэтому `Batch`, `Ref`, `Halt` и `BatchResult` — + протокол v1 — отображать не на что, а частичной ошибки, ради которой + существует `BatchError`, там не бывает. Отказ заменяет собой ответ портала на + тело v1-батча: `BITRIX_REST_V3_EXCEPTION_INVALIDSELECTEXCEPTION`, «Не удается + распознать выражение select» — сообщение про `select` на запрос, где его нет. + + Оба sentinel'а называют, чем пользоваться вместо них: `Core.Call` с + `pagination` и `Core.Call` с `batch`. + + На v1 не влияет: версия выводится из адреса, и обход с батчем на `/rest/` + работают как раньше. + +### Проверено на живом портале — REST 3.0 + +Всё ниже — ответы облачного портала, снятые вызовами самого SDK. + +- `.json`: `…/rest/api/1/токен/documentation` → 200, + `…/documentation.json` → 404 «Метод `documentation.json` не найден». +- Конверт успеха совпадает с v1: `humanresources.employee.count` → + `{"result":{"total":19},"time":{…}}`, `Unwrap(res.Result, "total")` = `19`. +- Формы ошибок (9 разных): вложенная — `…METHODNOTFOUNDEXCEPTION` (404), + `…VALIDATION_REQUESTVALIDATIONEXCEPTION` (400, с `validation[{field:"id"}]`), + `…ENTITYNOTFOUNDEXCEPTION`, `…UNKNOWNDTOPROPERTYEXCEPTION`, + `…INVALIDSELECTEXCEPTION`, `…INVALIDFILTEREXCEPTION`, `…INVALIDJSONEXCEPTION`, + `…ACCESSDENIEDEXCEPTION` (401 на неверный токен), а также + `CRM_EMAIL_INVALID_REQUEST` без приставки; плоская — `INTERNAL_SERVER_ERROR` + (500) на том же адресе v3. +- `errors.Is(err, ErrMethodNotFound)` = `true`, `CodeOf` = код v3 как пришёл. +- Списки: у `tasks.task.list` на v3 нет ни `next`, ни `total`; `start` + **молча игнорируется** (та же первая страница), фильтр v1 (`{">ID": …}`) + отклоняется. +- `batch`: работает в формате v3 через `Core.Call` (ответ — массив + `[{"total":19},{"items":[…]}]`); тело батча v1 отклоняется. +- `documentation`: 177 методов, 25 из них доступны по GET. Через `Call` даёт + `Result == nil` без ошибки — конверта у ответа нет. +- v1 на том же портале не изменился: `profile`, `batch`, `ERROR_METHOD_NOT_FOUND`. + +Не проверялось: OAuth-авторизация на v3 и `CallMultipart` на v3 (прогон шёл на +вебхуке; v3 заявляет только JSON-тело). ## [0.1.0] — 2026-08-04 diff --git a/README.md b/README.md index 7e1f9d3..a3b7ebc 100644 --- a/README.md +++ b/README.md @@ -571,6 +571,106 @@ _, err := client.App().InstallFinish(ctx, nil) Используйте его только если сценарий действительно требует серверного вызова. В стандартных сценариях метод вызывается во фронтенде. +## REST API 3.0 + +Чтобы вызывать методы [REST 3.0](https://apidocs.bitrix24.ru/api-reference/rest-v3.html), +достаточно передать адрес новой версии — с сегментом `/rest/api/` вместо +`/rest/`. Никакой опции для этого нет: + +```go +// v1: https://portal.bitrix24.ru/rest/1/токен/ +// v3: https://portal.bitrix24.ru/rest/api/1/токен/ +client := b24.NewClient("https://portal.bitrix24.ru/rest/api/1/токен/") + +res, err := client.Core().Call(ctx, "tasks.task.list", b24.Params{ + "select": []string{"id", "title"}, + "filter": [][]any{{"id", ">", 500}}, + "pagination": b24.Params{"limit": 20, "page": 1}, +}) +``` + +Версия выводится из адреса, а не задаётся отдельно, потому что адрес её и так +задаёт: без `/api/` портал выполнит метод старой версии или ответит «метод не +найден». Второй источник истины можно было бы рассогласовать с первым — сказать +«версия 3» и забыть `/api/` в адресе, — и тогда каждый вызов уходил бы в v1, а +разбирался бы по правилам v3. + +Для приложения адрес v3 — `https://portal.bitrix24.ru/rest/api/`, токен +по-прежнему уходит в теле запроса. + +### Что на v3 работает + +Проверено на живом портале (Битрикс24 облачный, август 2026): + +- **`Call`, `CallJSON`** — да. Конверт успешного ответа у v3 тот же, что у v1, + поэтому `CallResult`, `Result`, `Kind`, `Unwrap`, `UnwrapFold`, `IsEmpty`, `ID` + работают без изменений. +- **Коды ошибок** — да: `errors.Is`, `CodeOf` и sentinel-ошибки заполняются из + вложенного формата v3 (см. ниже). +- **Повторы и `WithIdempotent`** — да, логика та же. Ошибки инфраструктуры (в том + числе `QUERY_LIMIT_EXCEEDED`) на адресе v3 приходят в **плоском** формате v1, + и SDK разбирает оба. +- **`WithTimeout`, `WithHTTPClient`, `WithRetry`** — да, это транспорт, версии + не касается. + +### Что на v3 не работает + +- **`Pages` и `Scan`** — `ErrV3WalkUnsupported`. У v3 нет курсора: `start` + игнорируется, `next` и `total` в ответе отсутствуют, страница задаётся + параметром `pagination` (`page`, `limit`, `offset`). Обход не «портится», а + **отказывается стартовать** намеренно: на живом портале `Pages` по + `tasks.task.list` прочитал первую страницу, не увидел `next` и отчитался о + завершённом обходе с `Err() == nil` — 2 строки из 423. Частичная выгрузка, + выглядящая как полная, хуже ошибки. Листайте `Call`-ом: + + ```go + for page := 1; ; page++ { + res, err := client.Core().Call(ctx, "tasks.task.list", b24.Params{ + "select": []string{"id"}, + "pagination": b24.Params{"limit": 50, "page": page}, + }, b24.WithIdempotent()) + if err != nil { + return err + } + items, _ := b24.Unwrap(res.Result, "items") + // пусто — страницы кончились + } + ``` + +- **`Batch`, `CallBatch`, `CallBatchChunked`, `Ref`, `Halt`** — + `ErrV3BatchUnsupported`. Метод `batch` у v3 есть, но это другой протокол: + команды кладутся в **корень** тела как `{"method": …, "query": {…}}`, ответ — + **массив** в порядке отправки (ключи команд отбрасываются), а первая упавшая + команда обрывает весь запрос вместо `result_error`. Пока SDK не говорит на этом + формате, вызывайте его напрямую: + + ```go + res, err := client.Core().Call(ctx, "batch", b24.Params{ + "cnt": b24.Params{"method": "humanresources.employee.count", "query": b24.Params{}}, + "tsk": b24.Params{"method": "tasks.task.list", "query": b24.Params{"select": []string{"id"}}}, + }) + // res.Result = [{"total":19},{"items":[{"id":25}]}] — позиционно + ``` + +- **`CallMultipart`** — не проверялось. v3 заявляет только JSON-тело. +- **OAuth-авторизация на v3** — не проверялась: прогон шёл на вебхуке. Токен + уходит в теле, как и на v1, так что работать должно, но замера нет. + +### Список методов v3 — мимо SDK + +Портал отдаёт его сам, методом `documentation`, в формате OpenAPI. Но **через SDK +его брать нельзя**: этот метод отвечает самим документом, без конверта +`{"result": …}`, поэтому `Call` вернёт `Result == nil` и **никакой ошибки** — +запрос успешен, а данных нет. + +Берите его обычным HTTP-запросом: + +```go +resp, err := http.Get(webhookURL + "documentation") // адрес v3, GET, без параметров +``` + +На проверявшемся портале в документе 177 методов, из них 25 доступны и по GET. + ## Ошибки Ошибки, о которых сообщил портал, возвращаются как `*APIError` — с кодом, @@ -608,6 +708,53 @@ if errors.Is(err, b24.Code("CREATE_DYNAMIC_TYPE_RESTRICTED")) { … } // люб называют те же коды, а `b24.Code(...)` покрывает всё остальное — портал выпускает новые коды без предупреждения, поэтому набор намеренно открытый. +### Ошибки REST 3.0 + +У v3 другая форма ответа — код и текст лежат во вложенном объекте +(`{"error":{"code":…,"message":…}}`), а не плоско, — но снаружи это не видно: +`*APIError` заполняется из обеих форм, `errors.Is` и `CodeOf` работают как +прежде. Разбор идёт **по форме тела, а не по версии адреса**, потому что адрес v3 +отвечает обеими: ошибки шлюза (в том числе `QUERY_LIMIT_EXCEEDED`, на котором +держатся повторы) приходят в плоском формате v1 и на v3. + +Коды у версий разные, и **один** из них SDK сводит к старому — тот, у которого +на обеих версиях одно и то же значение: + +```go +// на адресе v3 это true, код на проводе — BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION +errors.Is(err, b24.ErrMethodNotFound) +``` + +Остальные не сводятся, и это не недоделка. `BITRIX_REST_V3_EXCEPTION_ACCESSDENIEDEXCEPTION` +похож на `ACCESS_DENIED`, но замер показал, что v3 отвечает им и на **неверный +токен вебхука**, где v1 отвечает `INVALID_CREDENTIALS`: один код v3 покрывает два +кода v1. Свести их — значит заставить ветку «права не те, авторизация в порядке» +срабатывать на протухшей авторизации. Поэтому для таких случаев — свои sentinel'ы +`ErrV3Validation`, `ErrV3EntityNotFound`, `ErrV3AccessDenied` и константы +`CodeV3*`; любой не перечисленный код по-прежнему берётся через `b24.Code(...)`. + +Приставка `BITRIX_REST_V3_EXCEPTION_` **не универсальна** — не выводите код из +неё. Замер: `crm.deal.timeline.activity.email.list` на плохой `id` отвечает +`CRM_EMAIL_INVALID_REQUEST`, в конверте v3 и без всякой приставки. + +`CodeOf` возвращает код **как он пришёл**, без перевода: он идёт в лог, и чужой +код там отправил бы читателя искать строку, которой портал не присылал. +Для ветвления — `errors.Is`, для лога — `CodeOf`. + +У ошибок валидации v3 есть то, чего у v1 нет вовсе: список полей, из-за которых +запрос отклонён. Код и текст у всех таких ошибок одинаково общие, так что без +него неизвестно, что именно не так: + +```go +var apiErr *b24.APIError +if errors.As(err, &apiErr) { + for _, v := range apiErr.Validation { + log.Printf("поле %s: %s", v.Field, v.Message) + // поле id: Обязательное поле `id` не указано + } +} +``` + ### Повторы: важно не «временная ли ошибка», а «выполнился ли запрос» - **`QUERY_LIMIT_EXCEEDED`** (HTTP 503) — это лимитер отказал в вызове **до diff --git a/batch.go b/batch.go index 050eb43..2405d3e 100644 --- a/batch.go +++ b/batch.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/bitrix24/b24gosdk/internal/phpq" + "github.com/bitrix24/b24gosdk/internal/rest" ) // MaxBatchCommands is the number of commands the server executes in one batch. @@ -31,6 +32,29 @@ var ErrBatchLengthExceeded = errors.New("b24gosdk: batch is longer than the serv // back. Every Ref error wraps it. var ErrBadRef = errors.New("b24gosdk: bad $result reference") +// ErrV3BatchUnsupported is returned by CallBatch and CallBatchChunked when the +// client addresses REST 3.0. +// +// REST 3.0 has a batch method, but not this batch: it takes each command as +// {"method": …, "query": {…}} at the top level of the body, answers with a plain +// ARRAY in submission order — the command ids are discarded — and aborts the +// whole request on the first failing command instead of reporting per-command +// errors in result_error. So Batch, Ref, Halt and BatchResult, which are the +// v1 protocol, have nothing to map onto, and the "partial failure" that +// BatchError exists for does not occur. +// +// The refusal replaces the portal's own answer to a v1 batch body, which is +// BITRIX_REST_V3_EXCEPTION_INVALIDSELECTEXCEPTION, "Не удается распознать +// выражение select" — a message about `select`, for a request that has none. +// +// Until the SDK speaks the v3 format, send it through Core.Call: +// +// res, err := client.Core().Call(ctx, "batch", b24gosdk.Params{ +// "cnt": b24gosdk.Params{"method": "humanresources.employee.count", "query": b24gosdk.Params{}}, +// }) +// // res.Result is [{"total":19}] — an array, positional +var ErrV3BatchUnsupported = errors.New("b24gosdk: Batch works with REST v1 only; REST 3.0 batch takes method/query commands and answers with a positional array — call it through Core.Call") + // CmdID identifies one command inside a batch. It is the key the results come // back under and the name a $result placeholder refers to. type CmdID string @@ -437,6 +461,8 @@ func (e *BatchError) sortedIDs() []CmdID { // so a batch of writes holds the connection for the sum of their durations — // half a minute for 50 crm.deal.add. Give ctx a deadline that fits, and see // Batch for what a timeout that fires costs. +// +// REST v1 only: on a v3 client it returns ErrV3BatchUnsupported. func (c *Core) CallBatch(ctx context.Context, b *Batch) (*BatchResult, error) { if c == nil || c.client == nil { return nil, fmt.Errorf("b24gosdk: core is nil") @@ -444,6 +470,9 @@ func (c *Core) CallBatch(ctx context.Context, b *Batch) (*BatchResult, error) { if b == nil || len(b.cmds) == 0 { return nil, errors.New("b24gosdk: batch: no commands; an empty batch spends a rate-limit token for nothing") } + if rest.IsV3(c.BaseURL()) { + return nil, ErrV3BatchUnsupported + } if len(b.cmds) > MaxBatchCommands { return nil, fmt.Errorf("%w: %d commands, the server takes at most %d; use CallBatchChunked", ErrBatchLengthExceeded, len(b.cmds), MaxBatchCommands) diff --git a/core.go b/core.go index 9c9f344..e84e3fd 100644 --- a/core.go +++ b/core.go @@ -18,12 +18,29 @@ import ( // token, along with HTTP status 401. const expiredTokenCode = "expired_token" +// ValidationError names one field a REST 3.0 request was rejected over. +// +// REST 3.0 answers a bad request with a single generic code and message — +// BITRIX_REST_V3_EXCEPTION_VALIDATION_REQUESTVALIDATIONEXCEPTION, "Ошибка при +// валидации объекта запроса" — and puts the only part a caller can act on, the +// field, in a separate array. Without it the error says that something in the +// request was wrong but not what. +type ValidationError struct { + Field string + Message string +} + // APIError represents an error returned by the Bitrix24 REST API. type APIError struct { Code string Description string HTTPStatus int RawBody string + + // Validation carries the per-field details of a REST 3.0 request + // validation error. Empty for every other error and for all of REST v1, + // which has no equivalent. + Validation []ValidationError } func (e *APIError) Error() string { @@ -443,12 +460,16 @@ func wrapAPIError(err error) error { return nil } if apiErr, ok := err.(*rest.APIError); ok { - return &APIError{ + out := &APIError{ Code: apiErr.Code, Description: apiErr.Description, HTTPStatus: apiErr.HTTPStatus, RawBody: apiErr.RawBody, } + for _, item := range apiErr.Validation { + out.Validation = append(out.Validation, ValidationError{Field: item.Field, Message: item.Message}) + } + return out } return err } diff --git a/doc.go b/doc.go index b6f9848..5a86249 100644 --- a/doc.go +++ b/doc.go @@ -86,6 +86,23 @@ // of a batch of adds, one entry per command so the gaps left by the ones that // failed do not shift the rest. // +// # REST 3.0 +// +// Pass a base URL with the /rest/api/ segment and calls go to REST 3.0. Nothing +// else is needed and there is no version option: the URL states the version +// already, since without /api/ the portal runs the v1 method of that name. +// +// client := b24gosdk.NewClient("https://portal.bitrix24.ru/rest/api/1/TOKEN/") +// +// The success envelope is the v1 one, so Call, Result, Unwrap and the rest are +// unchanged; error codes arrive nested and are parsed into the same *APIError, +// with Validation carrying the fields a request was rejected over. +// +// Pages, Scan, CallBatch and CallBatchChunked do NOT work on v3 and refuse with +// ErrV3WalkUnsupported and ErrV3BatchUnsupported rather than half-work — v3 +// paginates on its own pagination parameter and has a different batch protocol. +// Both sentinels name what to call instead. +// // # Errors // // Errors reported by the API are returned as *APIError. Match a code with diff --git a/errors.go b/errors.go index a238d17..f6e5b3a 100644 --- a/errors.go +++ b/errors.go @@ -48,6 +48,28 @@ const ( CodeBatchMethodNotAllow ErrorCode = "ERROR_BATCH_METHOD_NOT_ALLOWED" ) +// REST 3.0 codes, all met on a live portal. +// +// They are their own constants rather than new spellings of the ones above +// because most of them describe a condition v1 has no code for at all: v1 +// rejects a bad parameter with whatever the module felt like saying, v3 always +// with a validation error naming the field. +// +// The BITRIX_REST_V3_EXCEPTION_ prefix is NOT universal, so do not derive a +// code from it: crm.deal.timeline.activity.email.list answers a bad id with +// CRM_EMAIL_INVALID_REQUEST, in the v3 envelope, with no prefix. Anything not +// listed here is still matchable — Code("SOME_NEW_CODE") takes any string. +const ( + CodeV3MethodNotFound ErrorCode = "BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION" + CodeV3Validation ErrorCode = "BITRIX_REST_V3_EXCEPTION_VALIDATION_REQUESTVALIDATIONEXCEPTION" + CodeV3EntityNotFound ErrorCode = "BITRIX_REST_V3_EXCEPTION_ENTITYNOTFOUNDEXCEPTION" + CodeV3AccessDenied ErrorCode = "BITRIX_REST_V3_EXCEPTION_ACCESSDENIEDEXCEPTION" + CodeV3UnknownDTOProperty ErrorCode = "BITRIX_REST_V3_EXCEPTION_UNKNOWNDTOPROPERTYEXCEPTION" + CodeV3InvalidSelect ErrorCode = "BITRIX_REST_V3_EXCEPTION_INVALIDSELECTEXCEPTION" + CodeV3InvalidFilter ErrorCode = "BITRIX_REST_V3_EXCEPTION_INVALIDFILTEREXCEPTION" + CodeV3InvalidJSON ErrorCode = "BITRIX_REST_V3_EXCEPTION_INVALIDJSONEXCEPTION" +) + // Frequently matched codes as ready sentinels. var ( ErrQueryLimitExceeded = Code(CodeQueryLimitExceeded) @@ -59,8 +81,39 @@ var ( ErrMethodNotFound = Code(CodeMethodNotFound) ErrAccessDenied = Code(CodeAccessDenied) ErrPaymentRequired = Code(CodePaymentRequired) + + // REST 3.0 conditions with no v1 counterpart to fold into. + ErrV3Validation = Code(CodeV3Validation) + ErrV3EntityNotFound = Code(CodeV3EntityNotFound) + ErrV3AccessDenied = Code(CodeV3AccessDenied) ) +// v3Aliases folds a REST 3.0 code onto the v1 code for the SAME condition, so +// that errors.Is keeps working when a caller moves to a v3 URL. +// +// An entry is added only when both versions have been seen answering the same +// SITUATION, not merely when the two names read alike. That rule is why the +// table has one entry and not eight: +// +// - Method not found qualifies. A method name the portal does not know +// answers ERROR_METHOD_NOT_FOUND on v1 and +// BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION on v3 — same situation, +// same meaning. +// +// - Access denied does NOT, even though the names match. A wrong webhook +// token answers INVALID_CREDENTIALS on v1 but +// BITRIX_REST_V3_EXCEPTION_ACCESSDENIEDEXCEPTION on v3: v3 spends one code +// where v1 spends two. Folding it onto ACCESS_DENIED would make +// errors.Is(err, ErrAccessDenied) fire on a bad token — a branch that on v1 +// means "the credentials are fine, the rights are not". Match it as itself, +// with ErrV3AccessDenied. +// +// Keys must be written already normalized; TestV3AliasKeysAreNormalized holds +// that, because an unnormalized key would never be looked up. +var v3Aliases = map[ErrorCode]ErrorCode{ + CodeV3MethodNotFound: CodeMethodNotFound, +} + // errCode is the sentinel type Code returns. It is unexported so the only way to // build one is Code, which normalizes — a sentinel with an unnormalized code // would silently never match. @@ -85,9 +138,16 @@ func Code(c ErrorCode) error { return errCode(c.Normalize()) } // CodeOf reports the Bitrix24 error code an error carries, if any. // +// The code is the one that ARRIVED, normalized in case only. On REST 3.0 that +// is the v3 spelling: a missing method gives +// BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION here, while +// errors.Is(err, ErrMethodNotFound) is true for the same error. The two answer +// different questions — what the portal said, and what condition it was — so +// prefer errors.Is for branching and CodeOf for logging. +// // ok is false for an error that is not an *APIError, and for an *APIError whose // body carried no code — which happens when a proxy answers instead of the -// portal. Compare with the normalized constants, or use errors.Is with Code. +// portal. func CodeOf(err error) (ErrorCode, bool) { var apiErr *APIError if errors.As(err, &apiErr) && apiErr.Code != "" { @@ -105,6 +165,9 @@ func CodeOf(err error) (ErrorCode, bool) { // Matching is on the CODE ALONE, never on the HTTP status: OVERLOAD_LIMIT and // QUERY_LIMIT_EXCEEDED both arrive as 503, so a status-based match would treat a // manual block as a rate limit and retry something that must not be retried. +// +// A REST 3.0 code additionally matches the v1 sentinel for the same condition; +// see v3Aliases for which, and for why that list is short. func (e *APIError) Is(target error) bool { var want errCode if !errors.As(target, &want) { @@ -113,5 +176,10 @@ func (e *APIError) Is(target error) bool { if e == nil || e.Code == "" { return false } - return ErrorCode(e.Code).Normalize() == ErrorCode(want) + got := ErrorCode(e.Code).Normalize() + if got == ErrorCode(want) { + return true + } + alias, ok := v3Aliases[got] + return ok && alias.Normalize() == ErrorCode(want) } diff --git a/example_test.go b/example_test.go index 17ec7c4..07304c2 100644 --- a/example_test.go +++ b/example_test.go @@ -337,6 +337,49 @@ func ExampleAPIError() { } } +// Calling REST 3.0. The base URL alone selects the version: /rest/api/ instead +// of /rest/. Filters are arrays there and paging goes through the pagination +// parameter rather than start/next — which is why Pages and Scan refuse a v3 +// client instead of walking one page and calling the list finished. +func ExampleNewClient_restV3() { + // v1 would be https://portal.bitrix24.ru/rest/1/TOKEN/ + client := b24.NewClient("https://portal.bitrix24.ru/rest/api/1/TOKEN/") + + for page := 1; ; page++ { + res, err := client.Core().Call(context.Background(), "tasks.task.list", b24.Params{ + "select": []string{"id", "title"}, + "filter": [][]any{{"id", ">", 500}}, + "pagination": b24.Params{"limit": 50, "page": page}, + }, b24.WithIdempotent()) + if err != nil { + log.Fatal(err) + } + + items, ok := b24.Unwrap(res.Result, "items") + if !ok || b24.IsEmpty(items) { + break // v3 sends no next: an empty page is the end of the list + } + fmt.Println(string(items)) + } +} + +// A REST 3.0 validation error. Its code and message are the same generic pair +// for every rejected request, so the field names in Validation are the only +// part that says what was actually wrong. +func ExampleAPIError_validation() { + client := b24.NewClient("https://portal.bitrix24.ru/rest/api/1/TOKEN/") + + _, err := client.Core().Call(context.Background(), "tasks.task.get", nil, b24.WithIdempotent()) + if errors.Is(err, b24.ErrV3Validation) { + var apiErr *b24.APIError + if errors.As(err, &apiErr) { + for _, v := range apiErr.Validation { + fmt.Printf("field %s: %s\n", v.Field, v.Message) + } + } + } +} + // Asking what shape arrived before decoding it. ONE field answers with more // than one shape, and which one depends on the data: a single-value product // property is an object, a multiple one an array of the same objects. diff --git a/internal/rest/client.go b/internal/rest/client.go index 370579c..c8559df 100644 --- a/internal/rest/client.go +++ b/internal/rest/client.go @@ -43,12 +43,22 @@ type Response struct { Time json.RawMessage } +// ValidationError is one entry of the REST 3.0 error.validation array. +type ValidationError struct { + Field string + Message string +} + // APIError represents an error returned by the Bitrix24 REST API. type APIError struct { Code string Description string HTTPStatus int RawBody string + + // Validation carries the per-field details REST 3.0 attaches to a request + // validation error. Empty for every other error and for all of v1. + Validation []ValidationError } func (e *APIError) Error() string { @@ -264,8 +274,7 @@ func (c *Client) do(ctx context.Context, method string, body io.Reader, contentT // the body: QUERY_LIMIT_EXCEEDED comes with 503, expired_token with 401. apiErr := &APIError{HTTPStatus: resp.StatusCode, RawBody: string(bodyBytes)} if unmarshalErr == nil { - apiErr.Code = apiResp.Error - apiErr.Description = apiResp.ErrorDescription + apiErr.Code, apiErr.Description, apiErr.Validation = apiResp.errorFields() } return nil, apiErr } @@ -274,8 +283,8 @@ func (c *Client) do(ctx context.Context, method string, body io.Reader, contentT return nil, fmt.Errorf("b24: decode response: %w", unmarshalErr) } - if apiResp.Error != "" { - return nil, &APIError{Code: apiResp.Error, Description: apiResp.ErrorDescription, HTTPStatus: resp.StatusCode, RawBody: string(bodyBytes)} + if code, description, validation := apiResp.errorFields(); code != "" { + return nil, &APIError{Code: code, Description: description, Validation: validation, HTTPStatus: resp.StatusCode, RawBody: string(bodyBytes)} } return &Response{ @@ -287,14 +296,67 @@ func (c *Client) do(ctx context.Context, method string, body io.Reader, contentT } type envelope struct { - Result json.RawMessage `json:"result"` - Next json.RawMessage `json:"next"` - Total json.RawMessage `json:"total"` - Time json.RawMessage `json:"time"` - Error string `json:"error"` + Result json.RawMessage `json:"result"` + Next json.RawMessage `json:"next"` + Total json.RawMessage `json:"total"` + Time json.RawMessage `json:"time"` + // Error is raw because the two REST versions put a different JSON TYPE + // here: v1 a string, v3 an object. Decoding it as either one would make the + // other a decode failure instead of an error report. + Error json.RawMessage `json:"error"` ErrorDescription string `json:"error_description"` } +// v3ErrorBody is the REST 3.0 error object. +type v3ErrorBody struct { + Code string `json:"code"` + Message string `json:"message"` + Validation []struct { + Field string `json:"field"` + Message string `json:"message"` + } `json:"validation"` +} + +// errorFields reads the error code and its description out of a response body. +// +// # Why the shape decides, and not the URL version +// +// The two versions report errors differently — v1 flat, +// {"error":"CODE","error_description":"…"}, v3 nested, +// {"error":{"code":"CODE","message":"…"}} — but a v3 URL answers in BOTH. On a +// live portal …/rest/api/…/tasks.task.get with an id of the wrong type answers +// HTTP 500 with the v1 flat body, because that failure is reported by the REST +// gateway in front of the v3 controller rather than by the controller itself. +// Choosing the parser by URL version would drop the code of exactly those +// errors — and QUERY_LIMIT_EXCEEDED, which drives the retry loop, comes from +// that same gateway. +// +// So the JSON type of "error" decides. This cannot change v1: v1 has only ever +// sent the string form, and a body it does not send cannot alter its behavior. +func (e *envelope) errorFields() (code, description string, validation []ValidationError) { + raw := bytes.TrimSpace(e.Error) + if len(raw) == 0 || string(raw) == "null" { + return "", "", nil + } + + if raw[0] == '{' { + var nested v3ErrorBody + if err := json.Unmarshal(raw, &nested); err != nil { + return "", "", nil + } + for _, item := range nested.Validation { + validation = append(validation, ValidationError{Field: item.Field, Message: item.Message}) + } + return nested.Code, nested.Message, validation + } + + var flat string + if err := json.Unmarshal(raw, &flat); err != nil { + return "", "", nil + } + return flat, e.ErrorDescription, nil +} + // optionalInt reads a numeric metadata field. A missing or unexpected value // yields nil: response metadata must never fail an otherwise valid call. func optionalInt(raw json.RawMessage) *int { @@ -347,9 +409,48 @@ func normalizeBaseURL(baseURL string) string { return baseURL } +// v3Marker is the path segment that tells a REST 3.0 base URL from a v1 one. +// +// v1 addresses a webhook as https://portal/rest/{user}/{token}/, v3 as +// https://portal/rest/api/{user}/{token}/ — the extra /api/ is the whole +// difference, and for an application it is https://portal/rest/api/ against +// https://portal/rest/. A numeric user id can never spell "api", so the segment +// is unambiguous. +const v3Marker = "/rest/api/" + +// IsV3 reports whether baseURL addresses REST 3.0. +// +// # Why the URL, and not an option +// +// The caller has to pass a v3 URL anyway: without /api/ the portal runs the v1 +// method of that name, or answers "method not found" for a v3-only one. So the +// URL already states the version, and a second place to state it — an option — +// could disagree with the first. Version 3 selected while /api/ is missing from +// the URL would mean every call goes to v1 with v3 rules applied to the reply. +func IsV3(baseURL string) bool { + return strings.Contains(baseURL, v3Marker) +} + +// buildURL turns a method name into the URL to POST to. +// +// v1 wants the .json suffix; v3 REJECTS it — measured on a live portal, +// …/rest/api/{user}/{token}/documentation answers 200 while the same path with +// .json answers 404 "Метод `documentation.json` не найден". So the suffix is +// appended only for v1. +// +// The version is derived from BaseURL on every call rather than remembered on +// the client, so that a BaseURL replaced after construction cannot leave a +// stale flag behind. +// +// On v3 the method is passed through untouched, including a .json a caller +// typed by habit: the portal then names the method it could not find, which +// says more than a silent rewrite would. func buildURL(baseURL, method string) string { method = strings.TrimSpace(method) method = strings.TrimPrefix(method, "/") + if IsV3(baseURL) { + return baseURL + method + } if strings.HasSuffix(method, ".json") { return baseURL + method } diff --git a/internal/rest/v3_test.go b/internal/rest/v3_test.go new file mode 100644 index 0000000..571366f --- /dev/null +++ b/internal/rest/v3_test.go @@ -0,0 +1,252 @@ +package rest + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +// The bodies below are copied byte for byte from a live portal +// (mp24.bitrix24.ru, 2026-08-07). Hand-written approximations would test the +// approximation instead of the portal. + +// TestV3BaseURLDropsTheJSONSuffix is the single reason REST 3.0 did not work +// through this SDK at all. v1 wants …/crm.deal.add.json; v3 answers 404 +// "Метод `documentation.json` не найден" for exactly the same suffix, so a +// client that appends it unconditionally cannot reach a single v3 method. +func TestV3BaseURLDropsTheJSONSuffix(t *testing.T) { + var gotPath atomic.Value + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath.Store(r.URL.Path) + _, _ = w.Write([]byte(`{"result":{"total":19}}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/api/1/tok") + if _, err := c.CallJSON(context.Background(), "humanresources.employee.count", nil, false); err != nil { + t.Fatalf("CallJSON: %v", err) + } + if want := "/rest/api/1/tok/humanresources.employee.count"; gotPath.Load() != want { + t.Errorf("path = %v, want %q", gotPath.Load(), want) + } +} + +// TestV1BaseURLKeepsTheJSONSuffix is the regression guard for the other half. +// The version is inferred from the URL, so a mistake in that inference is +// invisible until v1 — every existing user of this SDK — starts calling +// …/crm.deal.add without the suffix. +func TestV1BaseURLKeepsTheJSONSuffix(t *testing.T) { + var gotPath atomic.Value + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath.Store(r.URL.Path) + _, _ = w.Write([]byte(`{"result":42}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/1/tok") + if _, err := c.CallJSON(context.Background(), "crm.deal.add", nil, false); err != nil { + t.Fatalf("CallJSON: %v", err) + } + if want := "/rest/1/tok/crm.deal.add.json"; gotPath.Load() != want { + t.Errorf("path = %v, want %q", gotPath.Load(), want) + } +} + +func TestBuildURLPerVersion(t *testing.T) { + tests := []struct { + name, base, method, want string + }{ + {"v1 webhook", "https://x/rest/1/tok/", "crm.deal.add", "https://x/rest/1/tok/crm.deal.add.json"}, + {"v1 application", "https://x/rest/", "crm.deal.add", "https://x/rest/crm.deal.add.json"}, + {"v3 webhook", "https://x/rest/api/1/tok/", "tasks.task.list", "https://x/rest/api/1/tok/tasks.task.list"}, + {"v3 application", "https://x/rest/api/", "tasks.task.list", "https://x/rest/api/tasks.task.list"}, + // A method a caller spelled with the suffix out of v1 habit is passed + // through, so the portal can name what it could not find. Rewriting it + // would be the SDK guessing at intent. + {"v3 keeps a typed suffix", "https://x/rest/api/1/tok/", "tasks.task.list.json", "https://x/rest/api/1/tok/tasks.task.list.json"}, + {"v1 tolerates a typed suffix", "https://x/rest/1/tok/", "crm.deal.add.json", "https://x/rest/1/tok/crm.deal.add.json"}, + // The marker is a whole path segment: a portal or method that merely + // contains the letters "api" is still v1. + {"api in the host is not v3", "https://api.example/rest/1/tok/", "crm.deal.add", "https://api.example/rest/1/tok/crm.deal.add.json"}, + {"api in the method is not v3", "https://x/rest/1/tok/", "rest.api.list", "https://x/rest/1/tok/rest.api.list.json"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := buildURL(tt.base, tt.method); got != tt.want { + t.Errorf("buildURL(%q, %q) = %q, want %q", tt.base, tt.method, got, tt.want) + } + }) + } +} + +// TestV3NestedErrorFillsAPIError covers the second incompatibility: v3 nests +// the code in an object where v1 has a flat string, so the v1-only parser left +// Code empty and the whole taxonomy — errors.Is, CodeOf, the retry decision — +// had nothing to work with. +func TestV3NestedErrorFillsAPIError(t *testing.T) { + const body = `{"error":{"code":"BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION","message":"Метод ` + "`no.such.method`" + ` не найден"}}` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(body)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/api/1/tok") + _, err := c.CallJSON(context.Background(), "no.such.method", nil, false) + + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("error = %v, want *APIError", err) + } + if apiErr.Code != "BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION" { + t.Errorf("Code = %q, want the code from error.code", apiErr.Code) + } + if apiErr.Description == "" { + t.Error("Description is empty, want the text from error.message") + } + if apiErr.HTTPStatus != http.StatusNotFound { + t.Errorf("HTTPStatus = %d, want 404", apiErr.HTTPStatus) + } +} + +// TestV3ValidationDetailsSurvive keeps the only actionable part of the most +// common v3 error. Its code and message are the same generic pair for every bad +// request; the field that was wrong lives only in error.validation, so dropping +// that array leaves a caller told that something is wrong and not what. +func TestV3ValidationDetailsSurvive(t *testing.T) { + const body = `{"error":{"code":"BITRIX_REST_V3_EXCEPTION_VALIDATION_REQUESTVALIDATIONEXCEPTION","message":"Ошибка при валидации объекта запроса","validation":[{"message":"Обязательное поле ` + "`id`" + ` не указано","field":"id"}]}}` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(body)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/api/1/tok") + _, err := c.CallJSON(context.Background(), "tasks.task.get", nil, false) + + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("error = %v, want *APIError", err) + } + if len(apiErr.Validation) != 1 { + t.Fatalf("Validation = %+v, want one entry", apiErr.Validation) + } + if apiErr.Validation[0].Field != "id" || apiErr.Validation[0].Message == "" { + t.Errorf("Validation[0] = %+v, want field id with a message", apiErr.Validation[0]) + } +} + +// TestFlatErrorOnAV3URLStillParses is why the parser looks at the SHAPE of the +// body and not at the version of the URL. A v3 URL answers in BOTH shapes: the +// REST gateway in front of the v3 controller reports in the v1 flat form, so +// version-driven parsing would drop the code of every gateway error — including +// QUERY_LIMIT_EXCEEDED, which the retry loop depends on. +// +// Measured: …/rest/api/…/tasks.task.get with {"id":"abc"} answers HTTP 500 +// {"error":"INTERNAL_SERVER_ERROR","error_description":"Internal server error"}. +func TestFlatErrorOnAV3URLStillParses(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"INTERNAL_SERVER_ERROR","error_description":"Internal server error"}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/api/1/tok") + _, err := c.CallJSON(context.Background(), "tasks.task.get", nil, false) + + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("error = %v, want *APIError", err) + } + if apiErr.Code != "INTERNAL_SERVER_ERROR" { + t.Errorf("Code = %q, want INTERNAL_SERVER_ERROR", apiErr.Code) + } + if apiErr.Description != "Internal server error" { + t.Errorf("Description = %q, want the flat error_description", apiErr.Description) + } +} + +// TestRateLimitRetriesInBothErrorShapes protects the retry loop across the two +// shapes. retryable decides on the CODE, so a shape whose code does not get +// parsed silently turns "the limiter refused the call before it ran" — the one +// class that is always safe to repeat — into a hard failure. +func TestRateLimitRetriesInBothErrorShapes(t *testing.T) { + shapes := map[string]string{ + "flat": `{"error":"QUERY_LIMIT_EXCEEDED","error_description":"Too many requests"}`, + "nested": `{"error":{"code":"QUERY_LIMIT_EXCEEDED","message":"Too many requests"}}`, + } + for name, body := range shapes { + t.Run(name, func(t *testing.T) { + var attempts atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if attempts.Add(1) == 1 { + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = w.Write([]byte(body)) + return + } + _, _ = w.Write([]byte(`{"result":{"total":19}}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/api/1/tok") + c.Retry = fastRetry(3) + // idempotent=false on purpose: a rate limit is retried whatever the + // method does, because it never reached the method. + if _, err := c.CallJSON(context.Background(), "humanresources.employee.count", nil, false); err != nil { + t.Fatalf("CallJSON: %v", err) + } + if got := attempts.Load(); got != 2 { + t.Errorf("attempts = %d, want 2 (one refusal, one success)", got) + } + }) + } +} + +// TestErrorFieldThatCarriesNoCode guards the parser against reading a code out +// of something that is not one. A body whose "error" is absent, null or of an +// unexpected type must leave Code empty rather than invent a value: Code drives +// the retry decision, and a fabricated one could make the SDK repeat a write. +func TestErrorFieldThatCarriesNoCode(t *testing.T) { + bodies := map[string]string{ + "absent": `{"result":1}`, + "null": `{"result":1,"error":null}`, + "number": `{"error":500}`, + "object no code": `{"error":{"message":"boom"}}`, + "array": `{"error":["boom"]}`, + "empty flat code": `{"error":"","error_description":"boom"}`, + } + for name, body := range bodies { + t.Run(name, func(t *testing.T) { + var env envelope + if err := json.Unmarshal([]byte(body), &env); err != nil { + t.Fatalf("unmarshal fixture: %v", err) + } + if code, _, _ := env.errorFields(); code != "" { + t.Errorf("code = %q, want empty", code) + } + }) + } +} + +func TestIsV3(t *testing.T) { + tests := []struct { + base string + want bool + }{ + {"https://portal.bitrix24.ru/rest/api/1/tok/", true}, + {"https://portal.bitrix24.ru/rest/api/", true}, + {"https://portal.bitrix24.ru/rest/1/tok/", false}, + {"https://portal.bitrix24.ru/rest/", false}, + {"https://api.bitrix24.ru/rest/1/tok/", false}, + {"", false}, + } + for _, tt := range tests { + if got := IsV3(tt.base); got != tt.want { + t.Errorf("IsV3(%q) = %v, want %v", tt.base, got, tt.want) + } + } +} diff --git a/llms.txt b/llms.txt index 58af395..9fd39e0 100644 --- a/llms.txt +++ b/llms.txt @@ -202,6 +202,60 @@ Inbound install event and the application page: req, err := b24.ParseAppRequest(r) // POST data of the app page client, err := b24.NewClientFromAppRequest(req) +## REST 3.0 + +To call REST 3.0, pass a v3 base URL — `/rest/api/` in place of `/rest/`. There +is NO version option, and none is coming: the URL already states the version, +because without `/api/` the portal runs the v1 method of that name. + + client := b24.NewClient("https://portal.bitrix24.ru/rest/api/1/TOKEN/") + res, err := client.Core().Call(ctx, "tasks.task.list", b24.Params{ + "select": []string{"id", "title"}, + "filter": [][]any{{"id", ">", 500}}, // v3 filters are ARRAYS, not a map + "pagination": b24.Params{"limit": 20, "page": 1}, + }) + +Verified against a live portal: + +- `Call`, `CallJSON`: work. The SUCCESS envelope is identical to v1, so + `Result`, `Kind`, `Unwrap`, `IsEmpty`, `ID` need no change. +- Error codes: work. v3 nests them (`{"error":{"code":…,"message":…}}`); the SDK + parses both shapes, so `errors.Is` and `CodeOf` keep working. +- Retries, `WithIdempotent`, `WithTimeout`: unchanged. +- `Pages` / `Scan`: DO NOT WORK, and refuse with `ErrV3WalkUnsupported`. v3 has + no cursor — it ignores `start` and returns no `next`/`total`. Page with + `pagination` {`page`, `limit`, `offset`} through `Call` yourself, and stop when + a page comes back empty. +- `Batch` / `CallBatch`: DO NOT WORK, and refuse with `ErrV3BatchUnsupported`. + v3's batch is a different protocol — commands at the top level of the body as + `{"method": …, "query": {…}}`, reply a POSITIONAL ARRAY with the command ids + discarded, and the first failure aborts everything. Call it directly: + + res, err := client.Core().Call(ctx, "batch", b24.Params{ + "a": b24.Params{"method": "humanresources.employee.count", "query": b24.Params{}}, + }) // res.Result = [{"total":19}] + +- `documentation` (the OpenAPI list of v3 methods): do NOT fetch it through the + SDK. It answers with the document itself, with no `{"result": …}` envelope, so + `Call` returns `Result == nil` AND no error. Use a plain `http.Get`. +- OAuth on v3 and `CallMultipart` on v3: untested. Do not claim they work. + +v3 error codes are their own strings, not the v1 ones. One is folded so that +existing code keeps matching — `errors.Is(err, b24.ErrMethodNotFound)` is true +for `BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION`. The rest are not folded +on purpose: v3 answers a WRONG WEBHOOK TOKEN with +`…_ACCESSDENIEDEXCEPTION`, where v1 answers `INVALID_CREDENTIALS`, so folding it +onto `ACCESS_DENIED` would fire the "rights are wrong, credentials are fine" +branch on a dead token. Use `ErrV3Validation`, `ErrV3EntityNotFound`, +`ErrV3AccessDenied`, the `CodeV3*` constants, or `b24.Code("…")` for anything +else. Do NOT derive a code from the `BITRIX_REST_V3_EXCEPTION_` prefix: it is not +universal (`crm.deal.timeline.activity.email.list` answers +`CRM_EMAIL_INVALID_REQUEST` in the same envelope). + +`apiErr.Validation` carries the fields a v3 request was rejected over. The code +and message of a validation error are the same generic pair every time, so this +is the only part that says WHAT was wrong. + ## Traps that cost data, not just an error **A chained batch MUST set `Halt`.** If a producer command fails, its `$result` @@ -288,7 +342,8 @@ constants name the same codes; `b24.Code(...)` covers everything else, because the portal ships new codes without warning. SDK-level sentinels (not portal codes): `ErrCursorStalled`, `ErrNoRows`, -`ErrBatchLengthExceeded`, `ErrBadRef`. +`ErrBatchLengthExceeded`, `ErrBadRef`, `ErrV3WalkUnsupported`, +`ErrV3BatchUnsupported`. **Retry is decided by "did the request execute?", not by "was it transient?"** diff --git a/pager.go b/pager.go index 591825d..a77c81b 100644 --- a/pager.go +++ b/pager.go @@ -6,6 +6,8 @@ import ( "errors" "fmt" "strings" + + "github.com/bitrix24/b24gosdk/internal/rest" ) // DefaultPageSize is how many rows a list method returns per page. It is fixed @@ -25,6 +27,23 @@ var ErrCursorStalled = errors.New("b24gosdk: the cursor did not move") // ErrNoRows is returned when a page carries no row array where one was expected. var ErrNoRows = errors.New("b24gosdk: no row array in result") +// ErrV3WalkUnsupported is returned by Pages and Scan when the client addresses +// REST 3.0. Page through v3 with Core.Call and its pagination parameter. +// +// # Why a refusal rather than a best effort +// +// Because the best effort loses data quietly. Both walks are built on the v1 +// cursor protocol — send `start`, read back `next`, stop once `next` is gone — +// and v3 implements none of it: it paginates on pagination{page,limit,offset}, +// ignores `start`, and answers with neither `next` nor `total`. Measured on a +// live portal: Pages over tasks.task.list on a v3 URL read the first page, found +// no `next`, and reported a FINISHED walk, Err() == nil, after 2 rows out of the +// 423 the portal held. A partial export that looks complete is the one failure a +// walk must never have. Scan happens to fail loudly instead — it sends a v1 +// filter that v3 rejects — but a walk whose safety depends on which method it +// was pointed at is not a guarantee. +var ErrV3WalkUnsupported = errors.New("b24gosdk: Pages and Scan work with REST v1 only; REST 3.0 paginates with the pagination parameter and returns no next cursor") + // Pager walks a list method page by page. // // It is a struct with Next/Rows/Err rather than an iterator function, so an @@ -186,6 +205,8 @@ func WithCallOptions(opts ...CallOption) PageOption { // This is the right walk for a few pages. For tens of thousands of rows use Scan: // offset paging makes the server count past every skipped row, so the last pages // of a large list get slower and slower. +// +// REST v1 only: on a v3 client it returns ErrV3WalkUnsupported. func (c *Core) Pages(method string, params any, opts ...PageOption) (*Pager, error) { return c.newPager(method, params, modeOffset, opts) } @@ -199,6 +220,8 @@ func (c *Core) Pages(method string, params any, opts ...PageOption) (*Pager, err // // It requires a method that sorts and filters by an id field. Where the id is // not spelled "ID" both ways, pass WithIDField. +// +// REST v1 only: on a v3 client it returns ErrV3WalkUnsupported. func (c *Core) Scan(method string, params any, opts ...PageOption) (*Pager, error) { return c.newPager(method, params, modeScan, opts) } @@ -210,6 +233,9 @@ func (c *Core) newPager(method string, params any, mode pageMode, opts []PageOpt if strings.TrimSpace(method) == "" { return nil, errors.New("b24gosdk: empty method name") } + if rest.IsV3(c.BaseURL()) { + return nil, fmt.Errorf("b24gosdk: %s: %w", method, ErrV3WalkUnsupported) + } m, err := paramsAsMap(params) if err != nil { return nil, fmt.Errorf("b24gosdk: %s: %w", method, err) diff --git a/v3_test.go b/v3_test.go new file mode 100644 index 0000000..1a92d09 --- /dev/null +++ b/v3_test.go @@ -0,0 +1,290 @@ +package b24gosdk + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" +) + +// v3Portal answers every request with one canned body and status, at a base URL +// that says REST 3.0. +func v3Portal(t *testing.T, status int, body string) *Client { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + return NewClient(srv.URL + "/rest/api/1/tok") +} + +// TestV3MethodNotFoundMatchesTheV1Sentinel is the reason v3Aliases exists. +// Code strings differ between the versions — ERROR_METHOD_NOT_FOUND against +// BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION — so without the fold, code +// written against errors.Is stops recognizing the condition the moment its +// caller switches to a v3 URL, and silently takes the else branch. +func TestV3MethodNotFoundMatchesTheV1Sentinel(t *testing.T) { + c := v3Portal(t, http.StatusNotFound, + `{"error":{"code":"BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION","message":"Метод не найден"}}`) + + _, err := c.Core().Call(context.Background(), "no.such.method", nil) + if !errors.Is(err, ErrMethodNotFound) { + t.Errorf("errors.Is(err, ErrMethodNotFound) = false for %v", err) + } + // The fold is one-way and narrow: it must not make the error match every + // other sentinel too. + if errors.Is(err, ErrAccessDenied) || errors.Is(err, ErrQueryLimitExceeded) { + t.Error("the v3 code matched an unrelated sentinel") + } +} + +// TestCodeOfReportsTheWireCode fixes the division of labour between the two +// ways of asking. CodeOf answers "what did the portal say" and must stay +// literal — it is what ends up in a log or a bug report, and a translated code +// there sends the reader looking for a string the portal never sent. errors.Is +// answers "which condition is this" and folds. +func TestCodeOfReportsTheWireCode(t *testing.T) { + c := v3Portal(t, http.StatusNotFound, + `{"error":{"code":"BITRIX_REST_V3_EXCEPTION_METHODNOTFOUNDEXCEPTION","message":"Метод не найден"}}`) + + _, err := c.Core().Call(context.Background(), "no.such.method", nil) + code, ok := CodeOf(err) + if !ok || code != CodeV3MethodNotFound { + t.Errorf("CodeOf = %q, %v; want the v3 code as it arrived", code, ok) + } +} + +// TestV3AccessDeniedIsNotFoldedIntoV1AccessDenied pins a deliberate NON-entry +// in v3Aliases. The names read alike, but the conditions are not the same set: +// a wrong webhook token answers INVALID_CREDENTIALS on v1 and +// ...ACCESSDENIEDEXCEPTION on v3 (measured on a live portal). Folding it would +// make errors.Is(err, ErrAccessDenied) true for a bad token — a branch that on +// v1 means the credentials are valid and the rights are not, which is where a +// caller decides to give up instead of re-authorizing. +func TestV3AccessDeniedIsNotFoldedIntoV1AccessDenied(t *testing.T) { + c := v3Portal(t, http.StatusUnauthorized, + `{"error":{"code":"BITRIX_REST_V3_EXCEPTION_ACCESSDENIEDEXCEPTION","message":"Доступ запрещен"}}`) + + _, err := c.Core().Call(context.Background(), "humanresources.employee.count", nil) + if errors.Is(err, ErrAccessDenied) { + t.Error("the v3 access code was folded into ErrAccessDenied; v3 spends it where v1 spends INVALID_CREDENTIALS too") + } + if !errors.Is(err, ErrV3AccessDenied) { + t.Errorf("errors.Is(err, ErrV3AccessDenied) = false for %v", err) + } +} + +// TestV3AliasKeysAreNormalized: the fold is a map lookup on the normalized +// code, so a key written in any other case would never be found and the alias +// would quietly do nothing. +func TestV3AliasKeysAreNormalized(t *testing.T) { + for key := range v3Aliases { + if key.Normalize() != key { + t.Errorf("v3Aliases key %q is not normalized (want %q)", key, key.Normalize()) + } + } +} + +// TestV3ValidationDetailsReachTheCaller carries the field names through the +// public API. The generic code and message of a v3 validation error say only +// that the request was rejected; error.validation is the part naming which +// field, and it has no v1 equivalent to fall back on. +func TestV3ValidationDetailsReachTheCaller(t *testing.T) { + c := v3Portal(t, http.StatusBadRequest, + `{"error":{"code":"BITRIX_REST_V3_EXCEPTION_VALIDATION_REQUESTVALIDATIONEXCEPTION","message":"Ошибка при валидации объекта запроса","validation":[{"message":"Обязательное поле `+"`id`"+` не указано","field":"id"}]}}`) + + _, err := c.Core().Call(context.Background(), "tasks.task.get", nil) + if !errors.Is(err, ErrV3Validation) { + t.Fatalf("errors.Is(err, ErrV3Validation) = false for %v", err) + } + + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("error = %v, want *APIError", err) + } + if len(apiErr.Validation) != 1 || apiErr.Validation[0].Field != "id" { + t.Errorf("Validation = %+v, want one entry for field id", apiErr.Validation) + } +} + +// TestV1ErrorTaxonomyIsUnchanged is the regression guard for every existing +// user. Error parsing now looks at the JSON type of "error" instead of assuming +// a string, and that rewrite must leave the v1 flat form matching exactly as +// before. +func TestV1ErrorTaxonomyIsUnchanged(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"error":"ERROR_METHOD_NOT_FOUND","error_description":"Method not found!"}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/1/tok") + _, err := c.Core().Call(context.Background(), "no.such.method", nil) + + if !errors.Is(err, ErrMethodNotFound) { + t.Errorf("errors.Is(err, ErrMethodNotFound) = false for %v", err) + } + code, ok := CodeOf(err) + if !ok || code != CodeMethodNotFound { + t.Errorf("CodeOf = %q, %v; want ERROR_METHOD_NOT_FOUND", code, ok) + } + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("error = %v, want *APIError", err) + } + if apiErr.Description != "Method not found!" { + t.Errorf("Description = %q, want the flat error_description", apiErr.Description) + } + if apiErr.Validation != nil { + t.Errorf("Validation = %+v, want nil: v1 has no such section", apiErr.Validation) + } +} + +// TestPagesRefusesAV3Client is the guard against a partial export that reports +// success. v3 ignores `start` and returns no `next`, so the walk reads page one, +// concludes the list ended and stops with Err() == nil — measured on a live +// portal as 2 rows out of 423. An error at construction is the only outcome a +// caller cannot mistake for a finished walk. +func TestPagesRefusesAV3Client(t *testing.T) { + calls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + _, _ = w.Write([]byte(`{"result":{"items":[{"id":1},{"id":2}]}}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/api/1/tok") + for _, tc := range []struct { + name string + open func() (*Pager, error) + }{ + {"Pages", func() (*Pager, error) { return c.Core().Pages("tasks.task.list", nil) }}, + {"Scan", func() (*Pager, error) { return c.Core().Scan("tasks.task.list", nil) }}, + } { + t.Run(tc.name, func(t *testing.T) { + p, err := tc.open() + if !errors.Is(err, ErrV3WalkUnsupported) { + t.Errorf("err = %v, want ErrV3WalkUnsupported", err) + } + if p != nil { + t.Error("a Pager was returned; a caller could loop over it and read a truncated list") + } + }) + } + // The refusal happens before any request: spending a rate-limit token to + // learn that the walk cannot work is not an improvement. + if calls != 0 { + t.Errorf("requests = %d, want 0", calls) + } +} + +// TestPagesStillWorksOnV1 pins the other side of that refusal. The version is +// inferred from the URL, so a mistake there silently disables paging for every +// existing v1 caller. +func TestPagesStillWorksOnV1(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"result":[{"ID":"1"},{"ID":"2"}],"total":2}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/1/tok") + p, err := c.Core().Pages("crm.deal.list", nil) + if err != nil { + t.Fatalf("Pages: %v", err) + } + if !p.Next(context.Background()) { + t.Fatalf("Next = false, Err = %v", p.Err()) + } + if got := len(p.Rows()); got != 2 { + t.Errorf("rows = %d, want 2", got) + } +} + +// TestCallBatchRefusesAV3Client turns the portal's own answer — an +// INVALIDSELECTEXCEPTION complaining about `select`, for a batch body that has +// none — into an error that names the actual incompatibility. v3 does have a +// batch, but with a different request shape and a positional array reply, so +// Batch, Ref, Halt and BatchResult have nothing to decode. +func TestCallBatchRefusesAV3Client(t *testing.T) { + calls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + _, _ = w.Write([]byte(`{"result":[{"total":19}]}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/api/1/tok") + b := NewBatch() + if _, err := b.Add("humanresources.employee.count", nil); err != nil { + t.Fatalf("Add: %v", err) + } + + if _, err := c.Core().CallBatch(context.Background(), b); !errors.Is(err, ErrV3BatchUnsupported) { + t.Errorf("CallBatch err = %v, want ErrV3BatchUnsupported", err) + } + // CallBatchChunked runs chunks through CallBatch, so it must carry the same + // refusal out rather than swallow it as a per-command failure. + if _, err := c.Core().CallBatchChunked(context.Background(), b); !errors.Is(err, ErrV3BatchUnsupported) { + t.Errorf("CallBatchChunked err = %v, want ErrV3BatchUnsupported", err) + } + if calls != 0 { + t.Errorf("requests = %d, want 0", calls) + } +} + +// TestV3BatchThroughCall is the documented way round the refusal above, and the +// reason the refusal is not a dead end. The shape is what the portal accepted: +// commands at the top level of the body, results as an array in submission +// order with the keys thrown away. +func TestV3BatchThroughCall(t *testing.T) { + var got map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewDecoder(r.Body).Decode(&got) + _, _ = w.Write([]byte(`{"result":[{"total":19},{"items":[{"id":25}]}]}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/api/1/tok") + res, err := c.Core().Call(context.Background(), "batch", Params{ + "cnt": Params{"method": "humanresources.employee.count", "query": Params{}}, + "tsk": Params{"method": "tasks.task.list", "query": Params{"select": []string{"id"}}}, + }) + if err != nil { + t.Fatalf("Call: %v", err) + } + if _, ok := got["cnt"].(map[string]any)["query"]; !ok { + t.Errorf("request body = %v, want each command at the top level with a query", got) + } + if Result(res.Result).Kind() != KindArray { + t.Errorf("result kind = %v, want an array: v3 answers positionally", Result(res.Result).Kind()) + } +} + +// TestV3CallReachesTheMethodWithoutTheJSONSuffix checks the whole path from the +// public API down, not just buildURL. The suffix is added deep in the transport, +// and every v3 method 404s while it is there. +func TestV3CallReachesTheMethodWithoutTheJSONSuffix(t *testing.T) { + paths := make(chan string, 1) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths <- r.URL.Path + _, _ = w.Write([]byte(`{"result":{"total":19}}`)) + })) + defer srv.Close() + + c := NewClient(srv.URL + "/rest/api/1/tok") + res, err := c.Core().Call(context.Background(), "humanresources.employee.count", nil) + if err != nil { + t.Fatalf("Call: %v", err) + } + if got, want := <-paths, "/rest/api/1/tok/humanresources.employee.count"; got != want { + t.Errorf("path = %q, want %q", got, want) + } + // The v3 success envelope is the v1 one, so Result, Unwrap and friends keep + // working; only the error half and the URL differ. + total, ok := Unwrap(res.Result, "total") + if !ok || string(total) != "19" { + t.Errorf("Unwrap(total) = %s, %v; want 19", total, ok) + } +}