Skip to content

Commit f2262d0

Browse files
authored
Assorted CI fixes (#1072)
* fix: remove redundant cloned() call in iterator * fix: only run cleanup if stack name is set * fix: only run rustdoc verification against workspace packages * fix(test): avoid cross-test state pollution in streaming_send_data_error_is_ignored - Use tokio_unstable with UnhandledPanic::ShutdownRuntime to detect panics - Create isolated runtime per test to avoid global panic hook interference - Test verifies spawned tasks handle send_data errors without panicking - CI: enable tokio_unstable for lambda_runtime tests only
1 parent c9f487f commit f2262d0

File tree

5 files changed

+29
-33
lines changed

5 files changed

+29
-33
lines changed

.github/actions/rust-build/action.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,6 @@ runs:
2828
- name: Run tests
2929
if: ${{ inputs.run-tests == 'true' }}
3030
shell: bash
31+
env:
32+
RUSTFLAGS: ${{ inputs.package == 'lambda_runtime' && '--cfg tokio_unstable' || '' }}
3133
run: cargo test --all-features --verbose --package ${{ inputs.package }}

.github/workflows/check-docs.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,5 @@ jobs:
3737
- name: Check documentation
3838
shell: bash
3939
env:
40-
RUSTFLAGS: --cfg docsrs
4140
RUSTDOCFLAGS: --cfg docsrs -Dwarnings
42-
run: cargo doc --no-deps --document-private-items --all-features
41+
run: cargo doc --workspace --no-deps --document-private-items --all-features

.github/workflows/run-integration-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ jobs:
5555
TEST_ENDPOINT: ${{ steps.deploy_stack.outputs.TEST_ENDPOINT }}
5656
run: cd lambda-integration-tests && cargo test
5757
- name: cleanup
58-
if: always()
58+
if: always() && steps.deploy_stack.outputs.STACK_NAME
5959
env:
6060
AWS_REGION: ${{ secrets.AWS_REGION }}
6161
STACK_NAME: ${{ steps.deploy_stack.outputs.STACK_NAME }}

lambda-http/src/response.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ impl LambdaResponse {
9797
let cookies = headers
9898
.get_all(SET_COOKIE)
9999
.iter()
100-
.cloned()
101100
.map(|v| v.to_str().ok().unwrap_or_default().to_string())
102101
.collect();
103102
headers.remove(SET_COOKIE);

lambda-runtime/src/requests.rs

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -218,45 +218,41 @@ mod tests {
218218
});
219219
}
220220

221-
#[tokio::test]
222-
async fn streaming_send_data_error_is_ignored() {
221+
#[test]
222+
#[cfg(tokio_unstable)]
223+
fn streaming_send_data_error_is_ignored() {
223224
use crate::StreamResponse;
224-
use std::sync::{
225-
atomic::{AtomicBool, Ordering},
226-
Arc,
227-
};
228-
229-
// Track if a panic occurred in any spawned task
230-
let panicked = Arc::new(AtomicBool::new(false));
231-
let panicked_clone = panicked.clone();
232225

233-
// Set a custom panic hook to detect panics in spawned tasks
234-
let old_hook = std::panic::take_hook();
235-
std::panic::set_hook(Box::new(move |_| {
236-
panicked_clone.store(true, Ordering::SeqCst);
237-
}));
238-
239-
let stream = tokio_stream::iter(vec![Ok::<Bytes, Error>(Bytes::from_static(b"chunk"))]);
226+
let runtime = tokio::runtime::Builder::new_current_thread()
227+
.unhandled_panic(tokio::runtime::UnhandledPanic::ShutdownRuntime)
228+
.enable_all()
229+
.build()
230+
.unwrap();
240231

241-
let stream_response: StreamResponse<_> = stream.into();
242-
let response = FunctionResponse::StreamingResponse(stream_response);
232+
// We don't want to use a global panic hook to avoid shared state across tests, but we need to know
233+
// if a background task panics. We accomplish this by using UnhandledPanic::ShutdownRuntime
234+
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
235+
runtime.block_on(async {
236+
let stream = tokio_stream::iter(vec![Ok::<Bytes, Error>(Bytes::from_static(b"chunk"))]);
243237

244-
let req: EventCompletionRequest<'_, _, (), _, _, _> = EventCompletionRequest::new("id", response);
238+
let stream_response: StreamResponse<_> = stream.into();
239+
let response = FunctionResponse::StreamingResponse(stream_response);
245240

246-
let http_req = req.into_req().expect("into_req should succeed");
241+
let req: EventCompletionRequest<'_, _, (), _, _, _> = EventCompletionRequest::new("id", response);
247242

248-
// immediate drop simulates client disconnection
249-
drop(http_req);
243+
let http_req = req.into_req().expect("into_req should succeed");
250244

251-
// give the spawned task time to run and potentially panic
252-
tokio::time::sleep(Duration::from_millis(10)).await;
245+
// immediate drop simulates client disconnection
246+
drop(http_req);
253247

254-
// Restore the old panic hook
255-
std::panic::set_hook(old_hook);
248+
// give the spawned task time to complete
249+
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
250+
})
251+
}));
256252

257253
assert!(
258-
!panicked.load(Ordering::SeqCst),
259-
"spawned task panicked - send_data errors should be ignored, not unwrapped"
254+
result.is_ok(),
255+
"spawned task panicked - send_data errors should be ignored"
260256
);
261257
}
262258
}

0 commit comments

Comments
 (0)