Skip to content

Commit d5b3d12

Browse files
committed
Fix a bug where subsequent requests would fail after a failed CSV import on postgres
fixes sqlpage#788
1 parent 5f006cc commit d5b3d12

File tree

7 files changed

+133
-48
lines changed

7 files changed

+133
-48
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
- Add support for HTTP Basic Authentication in the [fetch](https://sql-page.com/documentation.sql?component=fetch#component) function.
66
- Fix a bug where the table component would not add the right css classes to table cells.
7+
- Better error messages when a CSV import fails (using a `copy` statement and a file upload).
8+
- Fix a bug where subsequent requests would fail after a failed CSV import on postgres (https://github.com/sqlpage/SQLPage/issues/788)
79

810
## 0.32.1 (2025-01-03)
911

Cargo.lock

+33-33
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ panic = "abort"
1818
codegen-units = 2
1919

2020
[dependencies]
21-
sqlx = { package = "sqlx-oldapi", version = "0.6.38", features = [
21+
sqlx = { package = "sqlx-oldapi", version = "0.6.39", features = [
2222
"any",
2323
"runtime-actix-rustls",
2424
"sqlite",

src/webserver/database/csv_import.rs

+35-13
Original file line numberDiff line numberDiff line change
@@ -146,15 +146,24 @@ pub(super) async fn run_csv_import(
146146
csv_import: &CsvImport,
147147
request: &RequestInfo,
148148
) -> anyhow::Result<()> {
149-
let file_path = request
149+
let named_temp_file = &request
150150
.uploaded_files
151151
.get(&csv_import.uploaded_file)
152-
.ok_or_else(|| anyhow::anyhow!("File not found"))?
153-
.file
154-
.path();
155-
let file = tokio::fs::File::open(file_path)
156-
.await
157-
.with_context(|| "opening csv")?;
152+
.ok_or_else(|| {
153+
anyhow::anyhow!(
154+
"The request does not contain a field named {:?} with an uploaded file.\n\
155+
Please check that :\n\
156+
- you have selected a file to upload, \n\
157+
- the form field name is correct.",
158+
csv_import.uploaded_file
159+
)
160+
})?
161+
.file;
162+
let file_path = named_temp_file.path();
163+
let file_name = file_path.file_name().unwrap_or_default();
164+
let file = tokio::fs::File::open(file_path).await.with_context(|| {
165+
format!("The CSV file {file_name:?} was uploaded correctly, but could not be opened",)
166+
})?;
158167
let buffered = tokio::io::BufReader::new(file);
159168
// private_get_mut is not supposed to be used outside of sqlx, but it is the only way to
160169
// access the underlying connection
@@ -165,9 +174,9 @@ pub(super) async fn run_csv_import(
165174
_ => run_csv_import_insert(db, csv_import, buffered).await,
166175
}
167176
.with_context(|| {
177+
let table_name = &csv_import.table_name;
168178
format!(
169-
"running CSV import from {} to {}",
170-
csv_import.uploaded_file, csv_import.table_name
179+
"{file_name:?} was uploaded correctly, but its records could not be imported into the table {table_name}"
171180
)
172181
})
173182
}
@@ -179,13 +188,26 @@ async fn run_csv_import_postgres(
179188
csv_import: &CsvImport,
180189
file: impl AsyncRead + Unpin + Send,
181190
) -> anyhow::Result<()> {
191+
log::debug!("Running CSV import with postgres");
182192
let mut copy_transact = db
183193
.copy_in_raw(csv_import.query.as_str())
184194
.await
185-
.with_context(|| "running COPY IN")?;
186-
copy_transact.read_from(file).await?;
187-
copy_transact.finish().await?;
188-
Ok(())
195+
.with_context(|| "The postgres COPY FROM STDIN command failed.")?;
196+
log::debug!("Copy transaction created");
197+
match copy_transact.read_from(file).await {
198+
Ok(_) => {
199+
log::debug!("Copy transaction finished successfully");
200+
copy_transact.finish().await?;
201+
Ok(())
202+
}
203+
Err(e) => {
204+
log::debug!("Copy transaction failed with error: {e}");
205+
copy_transact
206+
.abort("The COPY FROM STDIN command failed.")
207+
.await?;
208+
Err(e.into())
209+
}
210+
}
189211
}
190212

191213
async fn run_csv_import_insert(

src/webserver/database/execute_queries.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ pub fn stream_query_results_with_conn<'a>(
5454
ParsedStatement::CsvImport(csv_import) => {
5555
let connection = take_connection(&request.app_state.db, db_connection).await?;
5656
log::debug!("Executing CSV import: {:?}", csv_import);
57-
run_csv_import(connection, csv_import, request).await?;
57+
run_csv_import(connection, csv_import, request).await.with_context(|| format!("Failed to import the CSV file {:?} into the table {:?}", csv_import.uploaded_file, csv_import.table_name))?;
5858
},
5959
ParsedStatement::StmtWithParams(stmt) => {
6060
let query = bind_parameters(stmt, request, db_connection).await?;

tests/index.rs

+59
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,65 @@ async fn test_official_website_basic_auth_example() {
636636
);
637637
}
638638

639+
/// https://github.com/sqlpage/SQLPage/issues/788
640+
#[actix_web::test]
641+
async fn test_failed_copy_followed_by_query() -> actix_web::Result<()> {
642+
let app_data = make_app_data().await;
643+
let big_csv = "col1,col2\nval1,val2\n".repeat(1000);
644+
let req = get_request_to_with_data(
645+
"/tests/sql_test_files/error_failed_to_import_the_csv.sql",
646+
app_data.clone(),
647+
)
648+
.await?
649+
.insert_header(("content-type", "multipart/form-data; boundary=1234567890"))
650+
.set_payload(format!(
651+
"--1234567890\r\n\
652+
Content-Disposition: form-data; name=\"recon_csv_file_input\"; filename=\"data.csv\"\r\n\
653+
Content-Type: text/csv\r\n\
654+
\r\n\
655+
{big_csv}\r\n\
656+
--1234567890--\r\n"
657+
))
658+
.to_srv_request();
659+
let resp = main_handler(req).await?;
660+
661+
assert_eq!(resp.status(), StatusCode::OK);
662+
let body = test::read_body(resp).await;
663+
let body_str = String::from_utf8(body.to_vec()).unwrap();
664+
assert!(
665+
body_str.contains("error"),
666+
"{body_str}\nexpected to contain error message"
667+
);
668+
669+
// On postgres, the error message should contain "The postgres COPY FROM STDIN command failed"
670+
if matches!(app_data.db.to_string().to_lowercase().as_str(), "postgres") {
671+
assert!(
672+
body_str.contains("The postgres COPY FROM STDIN command failed"),
673+
"{body_str}\nexpected to contain: The postgres COPY FROM STDIN command failed"
674+
);
675+
}
676+
// Now make other requests to verify the connection is still usable
677+
for path in [
678+
"/tests/sql_test_files/it_works_lower.sql",
679+
"/tests/sql_test_files/it_works_simple.sql",
680+
"/tests/sql_test_files/it_works_path.sql",
681+
] {
682+
let req = get_request_to_with_data(path, app_data.clone())
683+
.await?
684+
.to_srv_request();
685+
let resp = main_handler(req).await?;
686+
687+
assert_eq!(resp.status(), StatusCode::OK);
688+
let body = test::read_body(resp).await;
689+
let body_str = String::from_utf8(body.to_vec()).unwrap();
690+
assert!(
691+
body_str.contains("It works !"),
692+
"{body_str}\nexpected to contain: It works !"
693+
);
694+
}
695+
Ok(())
696+
}
697+
639698
async fn get_request_to_with_data(
640699
path: &str,
641700
data: actix_web::web::Data<AppState>,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
-- https://github.com/sqlpage/SQLPage/issues/788
2+
copy this_table_does_not_exist (csv) from 'recon_csv_file_input' DELIMITER '*' CSV;

0 commit comments

Comments
 (0)