Skip to content

Commit 8de3821

Browse files
goffrieConvex, Inc.
authored andcommitted
Don't validate in JsonPackedValue::from_network (#43147)
GitOrigin-RevId: 34302ceff8090693375e5d1ede4889882092a4ff
1 parent 2e4b244 commit 8de3821

File tree

17 files changed

+88
-68
lines changed

17 files changed

+88
-68
lines changed

crates/application/src/cache/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ impl<RT: Runtime> CacheManager<RT> {
604604
anyhow::bail!("Received non-query outcome when executing a query")
605605
};
606606
if let Ok(json_packed_value) = &query_outcome.result {
607-
let output: ConvexValue = json_packed_value.unpack();
607+
let output: ConvexValue = json_packed_value.unpack()?;
608608
let table_mapping = tx.table_mapping().namespace(component.into());
609609
let virtual_system_mapping = tx.virtual_system_mapping();
610610
let returns_validation_error = returns_validator.check_output(

crates/application/src/cron_jobs/mod.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -361,15 +361,15 @@ impl<RT: Runtime> CronJobContext<RT> {
361361
Ok(job_id)
362362
}
363363

364-
fn truncate_result(&self, result: JsonPackedValue) -> CronJobResult {
365-
let value = result.unpack();
364+
fn truncate_result(&self, result: JsonPackedValue) -> anyhow::Result<CronJobResult> {
365+
let value = result.unpack()?;
366366
let mut value_str = value.to_string();
367367
if value_str.len() <= CRON_LOG_MAX_RESULT_LENGTH {
368-
CronJobResult::Default(value)
368+
Ok(CronJobResult::Default(value))
369369
} else {
370370
value_str =
371371
value_str[..value_str.floor_char_boundary(CRON_LOG_MAX_RESULT_LENGTH)].to_string();
372-
CronJobResult::Truncated(value_str)
372+
Ok(CronJobResult::Truncated(value_str))
373373
}
374374
}
375375

@@ -466,7 +466,7 @@ impl<RT: Runtime> CronJobContext<RT> {
466466
let mut model = CronModel::new(&mut tx, component);
467467

468468
if let Ok(ref result) = outcome.result {
469-
let truncated_result = self.truncate_result(result.clone());
469+
let truncated_result = self.truncate_result(result.clone())?;
470470
let status = CronJobStatus::Success(truncated_result);
471471
model
472472
.insert_cron_job_log(
@@ -599,7 +599,7 @@ impl<RT: Runtime> CronJobContext<RT> {
599599

600600
let status = match completion.outcome.result.clone() {
601601
Ok(result) => {
602-
let truncated_result = self.truncate_result(result);
602+
let truncated_result = self.truncate_result(result)?;
603603
CronJobStatus::Success(truncated_result)
604604
},
605605
Err(e) => CronJobStatus::Err(e.to_string()),

crates/application/src/tests/components.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,10 @@ async fn test_env_vars_not_accessible_in_components(rt: TestRuntime) -> anyhow::
135135
application.commit_test(tx).await?;
136136
let result =
137137
run_function(&application, "componentEntry:envVarQuery".parse()?, vec![]).await??;
138-
assert_eq!(ConvexValue::Null, result.value.unpack());
138+
assert_eq!(ConvexValue::Null, result.value.unpack()?);
139139
let result =
140140
run_function(&application, "componentEntry:envVarAction".parse()?, vec![]).await??;
141-
assert_eq!(ConvexValue::Null, result.value.unpack());
141+
assert_eq!(ConvexValue::Null, result.value.unpack()?);
142142
Ok(())
143143
}
144144

@@ -152,14 +152,14 @@ async fn test_system_env_vars_not_accessible_in_components(rt: TestRuntime) -> a
152152
vec![],
153153
)
154154
.await??;
155-
assert_eq!(ConvexValue::Null, result.value.unpack());
155+
assert_eq!(ConvexValue::Null, result.value.unpack()?);
156156
let result = run_function(
157157
&application,
158158
"componentEntry:systemEnvVarAction".parse()?,
159159
vec![],
160160
)
161161
.await??;
162-
assert_eq!(ConvexValue::Null, result.value.unpack());
162+
assert_eq!(ConvexValue::Null, result.value.unpack()?);
163163
Ok(())
164164
}
165165

@@ -253,7 +253,7 @@ async fn test_date_now_within_component(rt: TestRuntime) -> anyhow::Result<()> {
253253
let result = run_function(&application, "componentEntry:dateNow".parse()?, vec![])
254254
.await??
255255
.value;
256-
must_let!(let ConvexValue::Array(dates) = result.unpack());
256+
must_let!(let ConvexValue::Array(dates) = result.unpack()?);
257257
assert_eq!(dates.len(), 2);
258258
must_let!(let ConvexValue::Float64(parent_date) = dates[0].clone());
259259
must_let!(let ConvexValue::Float64(child_date) = dates[1].clone());
@@ -273,7 +273,7 @@ async fn test_math_random_within_component(rt: TestRuntime) -> anyhow::Result<()
273273
let result = run_function(&application, "componentEntry:mathRandom".parse()?, vec![])
274274
.await??
275275
.value;
276-
must_let!(let ConvexValue::Array(randoms) = result.unpack());
276+
must_let!(let ConvexValue::Array(randoms) = result.unpack()?);
277277
assert_eq!(randoms.len(), 2);
278278
must_let!(let ConvexValue::Float64(parent_random) = randoms[0].clone());
279279
must_let!(let ConvexValue::Float64(child_random) = randoms[1].clone());

crates/application/src/tests/http_action.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,10 @@ async fn test_http_action_continues_after_client_disconnects(
420420
None,
421421
)
422422
.await?;
423-
assert_eq!(query_result.result.map(|v| v.unpack()), Ok(val!(true)));
423+
assert_eq!(
424+
query_result.result.map(|v| v.unpack().unwrap()),
425+
Ok(val!(true))
426+
);
424427

425428
Ok(())
426429
}

crates/application/src/tests/query_cache.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ async fn run_query_with_journal(
6868
let (function_log, _) = application.function_log().stream(0.0).await;
6969
let last_log_entry = function_log.last().unwrap();
7070
assert_eq!(last_log_entry.cached_result, expect_cached);
71-
Ok((result.result?.unpack(), result.journal))
71+
Ok((result.result?.unpack()?, result.journal))
7272
}
7373

7474
async fn run_query(
@@ -103,7 +103,7 @@ async fn insert_object(application: &Application<TestRuntime>) -> anyhow::Result
103103
None,
104104
)
105105
.await??;
106-
Ok(result.value.unpack())
106+
result.value.unpack()
107107
}
108108

109109
#[convex_macro::test_runtime]

crates/application/src/tests/storage.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ async fn test_storage_get_url(rt: TestRuntime) -> anyhow::Result<()> {
112112
None,
113113
)
114114
.await?;
115-
must_let!(let ConvexValue::String(url) = query_result.result?.unpack());
115+
must_let!(let ConvexValue::String(url) = query_result.result?.unpack()?);
116116
assert!(url.starts_with("http://127.0.0.1:8000/api/storage/"));
117117
// Call it from an action.
118118
let action_result = app
@@ -128,7 +128,7 @@ async fn test_storage_get_url(rt: TestRuntime) -> anyhow::Result<()> {
128128
caller.clone(),
129129
)
130130
.await??;
131-
must_let!(let ConvexValue::String(url) = action_result.value.unpack());
131+
must_let!(let ConvexValue::String(url) = action_result.value.unpack()?);
132132
assert!(url.starts_with("http://127.0.0.1:8000/api/storage/"));
133133

134134
// Now set a canonical url and call the functions again.
@@ -158,7 +158,7 @@ async fn test_storage_get_url(rt: TestRuntime) -> anyhow::Result<()> {
158158
None,
159159
)
160160
.await?;
161-
must_let!(let ConvexValue::String(url) = query_result.result?.unpack());
161+
must_let!(let ConvexValue::String(url) = query_result.result?.unpack()?);
162162
assert!(url.starts_with("https://carnitas.convex.cloud/api/storage/"));
163163
// Call it from an action.
164164
let action_result = app
@@ -174,7 +174,7 @@ async fn test_storage_get_url(rt: TestRuntime) -> anyhow::Result<()> {
174174
caller.clone(),
175175
)
176176
.await??;
177-
must_let!(let ConvexValue::String(url) = action_result.value.unpack());
177+
must_let!(let ConvexValue::String(url) = action_result.value.unpack()?);
178178
assert!(url.starts_with("https://carnitas.convex.cloud/api/storage/"));
179179

180180
Ok(())
@@ -212,7 +212,7 @@ async fn test_storage_generate_upload_url(rt: TestRuntime) -> anyhow::Result<()>
212212
None,
213213
)
214214
.await??;
215-
must_let!(let ConvexValue::String(url) = mutation_result.value.unpack());
215+
must_let!(let ConvexValue::String(url) = mutation_result.value.unpack()?);
216216
assert!(url.starts_with("http://127.0.0.1:8000/api/storage/upload?token="));
217217

218218
// Call it from an action.
@@ -229,7 +229,7 @@ async fn test_storage_generate_upload_url(rt: TestRuntime) -> anyhow::Result<()>
229229
caller.clone(),
230230
)
231231
.await??;
232-
must_let!(let ConvexValue::String(url) = action_result.value.unpack());
232+
must_let!(let ConvexValue::String(url) = action_result.value.unpack()?);
233233
assert!(url.starts_with("http://127.0.0.1:8000/api/storage/upload?token="));
234234

235235
// Now set a canonical url and call the functions again.
@@ -259,7 +259,7 @@ async fn test_storage_generate_upload_url(rt: TestRuntime) -> anyhow::Result<()>
259259
None,
260260
)
261261
.await??;
262-
must_let!(let ConvexValue::String(url) = mutation_result.value.unpack());
262+
must_let!(let ConvexValue::String(url) = mutation_result.value.unpack()?);
263263
assert!(url.starts_with("https://carnitas.convex.cloud/api/storage/upload?token="));
264264

265265
// Call it from an action.
@@ -276,7 +276,7 @@ async fn test_storage_generate_upload_url(rt: TestRuntime) -> anyhow::Result<()>
276276
caller.clone(),
277277
)
278278
.await??;
279-
must_let!(let ConvexValue::String(url) = action_result.value.unpack());
279+
must_let!(let ConvexValue::String(url) = action_result.value.unpack()?);
280280
assert!(url.starts_with("https://carnitas.convex.cloud/api/storage/upload?token="));
281281

282282
Ok(())

crates/isolate/src/environment/udf/async_syscall.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ impl<RT: Runtime> AsyncSyscallProvider<RT> for DatabaseUdfEnvironment<RT> {
610610
self.emit_sub_function_log_lines(path.for_logging(), log_lines);
611611

612612
let result = match result {
613-
Ok(r) => r.unpack(),
613+
Ok(r) => r.unpack()?,
614614
Err(e) => {
615615
// TODO: How do we want to propagate stack traces between component calls?
616616
anyhow::bail!(e);

crates/isolate/src/test_helpers.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -528,13 +528,13 @@ impl<RT: Runtime, P: Persistence> UdfTest<RT, P> {
528528
let value = outcome
529529
.result
530530
.as_ref()
531-
.map(|v| v.unpack())
532531
.map_err(|e| {
533532
anyhow::anyhow!(
534533
"mutation failed with user error. If that is intended, call mutation_js_error \
535534
or raw_mutation instead. {e:?}"
536535
)
537536
})
537+
.and_then(|v| v.unpack())
538538
.unwrap();
539539
Ok((value, outcome))
540540
}
@@ -685,13 +685,13 @@ impl<RT: Runtime, P: Persistence> UdfTest<RT, P> {
685685
let value = outcome
686686
.result
687687
.as_ref()
688-
.map(|v| v.unpack())
689688
.map_err(|e| {
690689
anyhow::anyhow!(
691690
"query failed with user error. If that is intended, call query_js_error or \
692691
raw_query instead. {e:?}"
693692
)
694693
})
694+
.and_then(|v| v.unpack())
695695
.unwrap();
696696
Ok((value, outcome))
697697
}
@@ -1096,13 +1096,13 @@ impl<RT: Runtime, P: Persistence> UdfTest<RT, P> {
10961096
let value = outcome
10971097
.result
10981098
.as_ref()
1099-
.map(|v| v.unpack())
11001099
.map_err(|e| {
11011100
anyhow::anyhow!(
11021101
"action failed with user error. If that is intended, call action_js_error or \
11031102
raw_action instead. {e:?}"
11041103
)
11051104
})
1105+
.and_then(|v| v.unpack())
11061106
.unwrap();
11071107
Ok((value, outcome, log_lines))
11081108
}

crates/isolate/src/tests/query.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,7 @@ fn unpack_pagination_result(
699699
t: &UdfTest<TestRuntime, TestPersistence>,
700700
outcome: &UdfOutcome,
701701
) -> (ConvexArray, bool, Cursor, String) {
702-
must_let!(let ConvexValue::Object(output) = outcome.result.clone().unwrap().unpack());
702+
must_let!(let ConvexValue::Object(output) = outcome.result.clone().unwrap().unpack().unwrap());
703703
must_let!(let Some(ConvexValue::Array(page)) = output.get("page"));
704704
must_let!(let Some(ConvexValue::Boolean(is_done)) = output.get("isDone"));
705705
must_let!(let Some(ConvexValue::String(cursor_string)) = output.get("continueCursor"));

crates/local_backend/src/canonical_urls.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,15 +270,15 @@ mod tests {
270270
.run_query("_system/frontend/convexCloudUrl".parse()?)
271271
.await?;
272272
assert_eq!(
273-
query_convex_cloud.result.map(|v| v.unpack()),
273+
query_convex_cloud.result.map(|v| v.unpack().unwrap()),
274274
Ok(val!("https://new-cloud.example.com"))
275275
);
276276

277277
let query_convex_site = backend
278278
.run_query("_system/frontend/convexSiteUrl".parse()?)
279279
.await?;
280280
assert_eq!(
281-
query_convex_site.result.map(|v| v.unpack()),
281+
query_convex_site.result.map(|v| v.unpack().unwrap()),
282282
Ok(val!("https://new-site.example.com"))
283283
);
284284

0 commit comments

Comments
 (0)