Skip to content

Commit 95885d8

Browse files
guybedfordguybedford
authored
fix: KVStore error handling (#1060)
1 parent 3a77536 commit 95885d8

19 files changed

+116
-61
lines changed

‎integration-tests/js-compute/fixtures/module-mode/src/kv-store.js

+17
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,26 @@ import {
88
} from './assertions.js';
99
import { KVStore } from 'fastly:kv-store';
1010
import { routes, isRunningLocally } from './routes.js';
11+
import { sdkVersion } from 'fastly:experimental';
12+
13+
const debug = sdkVersion.endsWith('-debug');
1114

1215
// kvstore e2e tests
1316
{
17+
routes.set('/kv-store/debug-error', async () => {
18+
if (!debug) return;
19+
20+
// special debug function to create a kv entry with an invalid handle
21+
const entry = fastly.dump(null, 'invalidkv');
22+
23+
// we can then test the invalid handle error
24+
try {
25+
await entry.text();
26+
} catch (e) {
27+
strictEqual(e.message.includes('Invalid handle'), true);
28+
}
29+
});
30+
1431
routes.set('/kv-store-e2e/list', async () => {
1532
const store = new KVStore('example-test-kv-store');
1633
try {

‎integration-tests/js-compute/fixtures/module-mode/tests.json

+1
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@
147147
"body": "hello world!"
148148
}
149149
},
150+
"GET /kv-store/debug-error": {},
150151
"GET /kv-store/delete/called-as-constructor": {},
151152
"GET /kv-store/delete/called-unbound": {},
152153
"GET /kv-store/delete/key-parameter-calls-7.1.17-ToString": {},

‎runtime/fastly/builtins/backend.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@
2828
#include "fastly.h"
2929

3030
using builtins::BuiltinImpl;
31+
using fastly::FastlyGetErrorMessage;
3132
using fastly::common::parse_and_validate_timeout;
3233
using fastly::fastly::Fastly;
33-
using fastly::fastly::FastlyGetErrorMessage;
3434
using fastly::fetch::RequestOrResponse;
3535
using fastly::secret_store::SecretStoreEntry;
3636

‎runtime/fastly/builtins/body.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
#include "js/Stream.h"
1616

1717
using builtins::web::streams::NativeStreamSource;
18+
using fastly::FastlyGetErrorMessage;
1819
using fastly::fastly::convertBodyInit;
19-
using fastly::fastly::FastlyGetErrorMessage;
2020
using fastly::fetch::RequestOrResponse;
2121

2222
namespace fastly::body {

‎runtime/fastly/builtins/cache-override.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
#include "host_api.h"
1414

1515
using builtins::BuiltinImpl;
16-
using fastly::fastly::FastlyGetErrorMessage;
16+
using fastly::FastlyGetErrorMessage;
1717

1818
namespace fastly::cache_override {
1919

‎runtime/fastly/builtins/cache-simple.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313

1414
using builtins::BuiltinNoConstructor;
1515
using builtins::web::streams::NativeStreamSource;
16+
using fastly::FastlyGetErrorMessage;
1617
using fastly::fastly::convertBodyInit;
17-
using fastly::fastly::FastlyGetErrorMessage;
1818
using fastly::fetch::RequestOrResponse;
1919

2020
namespace fastly::cache_simple {

‎runtime/fastly/builtins/config-store.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#include "fastly.h"
55

66
using builtins::BuiltinImpl;
7-
using fastly::fastly::FastlyGetErrorMessage;
7+
using fastly::FastlyGetErrorMessage;
88

99
namespace fastly::config_store {
1010

‎runtime/fastly/builtins/dictionary.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
#include "../host-api/host_api_fastly.h"
44
#include "fastly.h"
55

6-
using fastly::fastly::FastlyGetErrorMessage;
6+
using fastly::FastlyGetErrorMessage;
77

88
namespace fastly::dictionary {
99

‎runtime/fastly/builtins/fastly.cpp

+21-8
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "fastly.h"
1212
#include "js/Conversions.h"
1313
#include "js/JSON.h"
14+
#include "kv-store.h"
1415
#include "logger.h"
1516
#include <arpa/inet.h>
1617

@@ -38,14 +39,6 @@ bool debug_logging_enabled() { return DEBUG_LOGGING_ENABLED; }
3839

3940
namespace fastly::fastly {
4041

41-
const JSErrorFormatString *FastlyGetErrorMessage(void *userRef, unsigned errorNumber) {
42-
if (errorNumber > 0 && errorNumber < JSErrNum_Limit) {
43-
return &fastly_ErrorFormatString[errorNumber];
44-
}
45-
46-
return nullptr;
47-
}
48-
4942
JS::PersistentRooted<JSObject *> Fastly::env;
5043
JS::PersistentRooted<JSObject *> Fastly::baseURL;
5144
JS::PersistentRooted<JSString *> Fastly::defaultBackend;
@@ -57,6 +50,26 @@ bool Fastly::dump(JSContext *cx, unsigned argc, JS::Value *vp) {
5750
if (!args.requireAtLeast(cx, __func__, 1))
5851
return false;
5952

53+
// special debug mode operations
54+
#ifndef NDEBUG
55+
if (args.get(0).isNull() && args.get(1).isString()) {
56+
JS::RootedString str(cx, args.get(1).toString());
57+
auto str_chars = core::encode(cx, str);
58+
if (!str_chars) {
59+
return false;
60+
}
61+
if (!strcmp(str_chars.ptr.get(), "invalidkv")) {
62+
host_api::HttpBody body(-1);
63+
host_api::HostBytes metadata{};
64+
// uint32_t gen = std::get<2>(res.unwrap());
65+
JS::RootedObject entry(
66+
cx, ::fastly::kv_store::KVStoreEntry::create(cx, body, std::move(metadata)));
67+
args.rval().setObject(*entry);
68+
return true;
69+
}
70+
}
71+
#endif
72+
6073
ENGINE->dump_value(args[0], stdout);
6174

6275
args.rval().setUndefined();

‎runtime/fastly/builtins/fastly.h

-2
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ class Env : public builtins::BuiltinNoConstructor<Env> {
2626
static JSObject *create(JSContext *cx);
2727
};
2828

29-
const JSErrorFormatString *FastlyGetErrorMessage(void *userRef, unsigned errorNumber);
30-
3129
class Fastly : public builtins::BuiltinNoConstructor<Fastly> {
3230
private:
3331
static bool log(JSContext *cx, unsigned argc, JS::Value *vp);

‎runtime/fastly/builtins/fetch/fetch.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ using fastly::backend::Backend;
1111
using fastly::fastly::Fastly;
1212
using fastly::fetch::Request;
1313

14-
using fastly::fastly::FastlyGetErrorMessage;
14+
using fastly::FastlyGetErrorMessage;
1515

1616
namespace fastly::fetch {
1717

‎runtime/fastly/builtins/fetch/request-response.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ using builtins::web::streams::TransformStream;
5555
using builtins::web::url::URL;
5656
using builtins::web::url::URLSearchParams;
5757
using builtins::web::worker_location::WorkerLocation;
58+
using fastly::FastlyGetErrorMessage;
5859
using fastly::backend::Backend;
5960
using fastly::cache_core::CacheEntry;
6061
using fastly::cache_override::CacheOverride;
6162
using fastly::cache_simple::SimpleCacheEntry;
62-
using fastly::fastly::FastlyGetErrorMessage;
6363
using fastly::fetch_event::FetchEvent;
6464
using fastly::kv_store::KVStoreEntry;
6565

‎runtime/fastly/builtins/kv-store.cpp

+5-14
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@
2727
#include "kv-store.h"
2828

2929
using builtins::web::streams::NativeStreamSource;
30+
using fastly::FastlyGetErrorMessage;
3031
using fastly::common::parse_and_validate_timeout;
3132
using fastly::common::validate_bytes;
3233
using fastly::fastly::convertBodyInit;
33-
using fastly::fastly::FastlyGetErrorMessage;
3434
using fastly::fetch::RequestOrResponse;
3535

3636
namespace fastly::kv_store {
@@ -248,9 +248,7 @@ bool process_pending_kv_store_list(JSContext *cx, host_api::KVStorePendingList::
248248

249249
auto res = pending_list.wait();
250250
if (auto *err = res.to_err()) {
251-
std::string message = std::move(err->message()).value_or("when attempting to fetch resource.");
252-
JS_ReportErrorNumberASCII(cx, FastlyGetErrorMessage, nullptr, JSMSG_KV_STORE_LIST_ERROR,
253-
message.c_str());
251+
HANDLE_KV_ERROR(cx, *err, JSMSG_KV_STORE_LIST_ERROR);
254252
return RejectPromiseWithPendingError(cx, promise);
255253
}
256254

@@ -314,9 +312,7 @@ bool process_pending_kv_store_delete(JSContext *cx, host_api::KVStorePendingDele
314312

315313
auto res = pending_delete.wait();
316314
if (auto *err = res.to_err()) {
317-
std::string message = std::move(err->message()).value_or("when attempting to fetch resource.");
318-
JS_ReportErrorNumberASCII(cx, FastlyGetErrorMessage, nullptr, JSMSG_KV_STORE_DELETE_ERROR,
319-
message.c_str());
315+
HANDLE_KV_ERROR(cx, *err, JSMSG_KV_STORE_DELETE_ERROR);
320316
return RejectPromiseWithPendingError(cx, promise);
321317
}
322318

@@ -331,9 +327,7 @@ bool process_pending_kv_store_lookup(JSContext *cx, host_api::KVStorePendingLook
331327
auto res = pending_lookup.wait();
332328

333329
if (auto *err = res.to_err()) {
334-
std::string message = std::move(err->message()).value_or("when attempting to fetch resource.");
335-
JS_ReportErrorNumberASCII(cx, FastlyGetErrorMessage, nullptr, JSMSG_KV_STORE_LOOKUP_ERROR,
336-
message.c_str());
330+
HANDLE_KV_ERROR(cx, *err, JSMSG_KV_STORE_LOOKUP_ERROR);
337331
return RejectPromiseWithPendingError(cx, promise);
338332
}
339333

@@ -637,10 +631,7 @@ bool KVStore::put(JSContext *cx, unsigned argc, JS::Value *vp) {
637631

638632
auto res = pending_insert.wait();
639633
if (auto *err = res.to_err()) {
640-
std::string message =
641-
std::move(err->message()).value_or("when attempting to fetch resource.");
642-
JS_ReportErrorNumberASCII(cx, FastlyGetErrorMessage, nullptr, JSMSG_KV_STORE_LIST_ERROR,
643-
message.c_str());
634+
HANDLE_KV_ERROR(cx, *err, JSMSG_KV_STORE_INSERT_ERROR);
644635
return RejectPromiseWithPendingError(cx, result_promise);
645636
}
646637

‎runtime/fastly/builtins/kv-store.h

+3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
#include "./fetch/request-response.h"
66
#include "builtin.h"
77

8+
#define HANDLE_KV_ERROR(cx, err, err_type) \
9+
::host_api::handle_kv_error(cx, err, err_type, __LINE__, __func__)
10+
811
namespace fastly::kv_store {
912

1013
class KVStoreEntry final : public builtins::BuiltinImpl<KVStoreEntry> {

‎runtime/fastly/builtins/secret-store.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
#include "../host-api/host_api_fastly.h"
55
#include "fastly.h"
66

7+
using fastly::FastlyGetErrorMessage;
78
using fastly::common::validate_bytes;
8-
using fastly::fastly::FastlyGetErrorMessage;
99

1010
namespace fastly::secret_store {
1111

‎runtime/fastly/common/validations.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#include "../host-api/host_api_fastly.h"
55
#include "encode.h"
66

7-
using fastly::fastly::FastlyGetErrorMessage;
7+
using fastly::FastlyGetErrorMessage;
88

99
namespace fastly::common {
1010
std::optional<uint32_t> parse_and_validate_timeout(JSContext *cx, JS::HandleValue value,

‎runtime/fastly/host-api/host_api.cpp

+21-17
Original file line numberDiff line numberDiff line change
@@ -753,42 +753,45 @@ make_fastly_send_error(fastly::fastly_host_http_send_error_detail &send_error_de
753753
return res;
754754
}
755755

756-
FastlyKVError make_fastly_kv_error(fastly::fastly_kv_error kv_error) {
756+
FastlyKVError make_fastly_kv_error(fastly::fastly_kv_error kv_error,
757+
fastly::fastly_host_error host_err) {
757758
FastlyKVError err;
758759
switch (kv_error) {
759760
case KV_ERROR_BAD_REQUEST: {
760761
err.detail = FastlyKVError::detail::bad_request;
761-
break;
762+
return err;
762763
}
763764
case KV_ERROR_INTERNAL_ERROR: {
764765
err.detail = FastlyKVError::detail::internal_error;
765-
break;
766+
return err;
766767
}
767768
case KV_ERROR_NOT_FOUND: {
768769
err.detail = FastlyKVError::detail::not_found;
769-
break;
770+
return err;
770771
}
771772
case KV_ERROR_OK: {
772773
err.detail = FastlyKVError::detail::ok;
773-
break;
774+
return err;
774775
}
775776
case KV_ERROR_PAYLOAD_TOO_LARGE: {
776777
err.detail = FastlyKVError::detail::payload_too_large;
777-
break;
778+
return err;
778779
}
779780
case KV_ERROR_PRECONDITION_FAILED: {
780781
err.detail = FastlyKVError::detail::precondition_failed;
781-
break;
782+
return err;
782783
}
783784
case KV_ERROR_TOO_MANY_REQUESTS: {
784785
err.detail = FastlyKVError::detail::too_many_requests;
785-
break;
786+
return err;
786787
}
787788
case KV_ERROR_UNINITIALIZED: {
788789
err.detail = FastlyKVError::detail::uninitialized;
789-
break;
790+
return err;
790791
}
791792
}
793+
err.detail = FastlyKVError::detail::host_error;
794+
err.host_err = host_err;
792795
return err;
793796
}
794797

@@ -2523,8 +2526,9 @@ const std::optional<std::string> FastlyKVError::message() const {
25232526
/// The kv-error-detail struct has not been populated.
25242527
case uninitialized:
25252528
return "Uninitialized.";
2526-
/// There was no kv error.
2529+
/// Host error / no error
25272530
case ok:
2531+
case host_error:
25282532
return std::nullopt;
25292533
/// Bad request.
25302534
case bad_request:
@@ -3213,11 +3217,11 @@ FastlyResult<HttpBody, FastlyKVError> KVStorePendingList::wait() {
32133217

32143218
fastly::fastly_host_error err;
32153219
HttpBody body{};
3216-
fastly::fastly_kv_error kv_err;
3220+
fastly::fastly_kv_error kv_err = KV_ERROR_UNINITIALIZED;
32173221

32183222
if (!convert_result(fastly::kv_store_list_wait(this->handle, &body.handle, &kv_err), &err) ||
32193223
kv_err != KV_ERROR_OK || body.handle == INVALID_HANDLE) {
3220-
res.emplace_err(make_fastly_kv_error(kv_err));
3224+
res.emplace_err(make_fastly_kv_error(kv_err, err));
32213225
} else {
32223226
res.emplace(body);
32233227
}
@@ -3262,7 +3266,7 @@ KVStorePendingLookup::wait() {
32623266
&err) ||
32633267
((kv_err != KV_ERROR_OK || body.handle == INVALID_HANDLE) && kv_err != KV_ERROR_NOT_FOUND)) {
32643268
cabi_free(metadata_buf);
3265-
res.emplace_err(make_fastly_kv_error(kv_err));
3269+
res.emplace_err(make_fastly_kv_error(kv_err, err));
32663270
} else if (kv_err == KV_ERROR_NOT_FOUND) {
32673271
cabi_free(metadata_buf);
32683272
res.emplace(std::nullopt);
@@ -3301,10 +3305,10 @@ Result<KVStorePendingDelete::Handle> KVStore::delete_(std::string_view key) {
33013305
FastlyResult<Void, FastlyKVError> KVStorePendingDelete::wait() {
33023306
FastlyResult<Void, FastlyKVError> res;
33033307
fastly::fastly_host_error err;
3304-
fastly::fastly_kv_error kv_err;
3308+
fastly::fastly_kv_error kv_err = KV_ERROR_UNINITIALIZED;
33053309
if (!convert_result(fastly::kv_store_delete_wait(this->handle, &kv_err), &err) ||
33063310
kv_err != KV_ERROR_OK) {
3307-
res.emplace_err(make_fastly_kv_error(kv_err));
3311+
res.emplace_err(make_fastly_kv_error(kv_err, err));
33083312
}
33093313
return res;
33103314
}
@@ -3371,10 +3375,10 @@ KVStore::insert(std::string_view key, HttpBody body, std::optional<InsertMode> m
33713375
FastlyResult<Void, FastlyKVError> KVStorePendingInsert::wait() {
33723376
FastlyResult<Void, FastlyKVError> res;
33733377
fastly::fastly_host_error err;
3374-
fastly::fastly_kv_error kv_err;
3378+
fastly::fastly_kv_error kv_err = KV_ERROR_UNINITIALIZED;
33753379
if (!convert_result(fastly::kv_store_insert_wait(this->handle, &kv_err), &err) ||
33763380
kv_err != KV_ERROR_OK) {
3377-
res.emplace_err(make_fastly_kv_error(kv_err));
3381+
res.emplace_err(make_fastly_kv_error(kv_err, err));
33783382
}
33793383
return res;
33803384
}

0 commit comments

Comments
 (0)