Skip to content

Commit 7ed45d5

Browse files
authored
fix: prevent panic on duplicate tracing initialization (#718)
* fix: prevent panic on duplicate tracing initialization * feat: test for concurrent flight-sql client init * fix: review comment
1 parent 42f8fc0 commit 7ed45d5

2 files changed

Lines changed: 58 additions & 6 deletions

File tree

sdk/packages/flight-sql-client/src/lib.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use napi_derive::{module_exports, napi};
2020
use snafu::prelude::*;
2121
use tokio::sync::Mutex;
2222
use tonic::transport::Channel;
23-
use tracing::info;
23+
use tracing::{info, warn};
2424
use tracing_subscriber::EnvFilter;
2525

2626
use crate::conversion::record_batches_to_buffer;
@@ -29,14 +29,19 @@ use crate::flight_client::{execute_flight, setup_client, ClientOptions};
2929

3030
fn init_logging() {
3131
// Set up a subscriber that logs to stdout
32-
let subscriber = tracing_subscriber::FmtSubscriber::builder()
32+
if let Err(e) = tracing_subscriber::FmtSubscriber::builder()
3333
.with_env_filter(
3434
EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("info")),
3535
)
36-
.finish();
37-
38-
// Set the global default subscriber
39-
tracing::subscriber::set_global_default(subscriber).expect("Failed to set tracing subscriber");
36+
.try_init()
37+
{
38+
let error_msg = e.to_string();
39+
if error_msg.contains("already been set") {
40+
warn!("Tracing already initialized");
41+
} else {
42+
panic!("Failed to initialize tracing: {}", e);
43+
}
44+
}
4045
}
4146

4247
#[module_exports]

tests/suite/src/__tests__/correctness/fast/flight-sql.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,53 @@ describe('flight sql', () => {
4747
authenticatedDID = await randomDID()
4848
}, 20000)
4949

50+
test('concurrent initialization across threads', async () => {
51+
const { Worker } = await import('worker_threads');
52+
53+
interface WorkerMessage {
54+
success: boolean;
55+
error?: string;
56+
}
57+
58+
// Create workers that will all try to load the module
59+
const createWorker = (): Promise<WorkerMessage> => new Promise((resolve, reject) => {
60+
const worker = new Worker(`
61+
import { createFlightSqlClient } from '@ceramic-sdk/flight-sql-client';
62+
import { parentPort } from 'worker_threads';
63+
const OPTIONS = {
64+
headers: new Array(),
65+
username: undefined,
66+
password: undefined,
67+
token: undefined,
68+
tls: false,
69+
host: "",
70+
port: 0,
71+
}
72+
try {
73+
const client = createFlightSqlClient(OPTIONS);
74+
parentPort.postMessage({ success: true });
75+
} catch (error) {
76+
parentPort.postMessage({ success: false, error: error.message });
77+
}
78+
`, {
79+
eval: true,
80+
});
81+
82+
worker.on('message', (data) => resolve(data as WorkerMessage));
83+
worker.on('error', reject);
84+
});
85+
86+
const results = await Promise.all([
87+
createWorker(),
88+
createWorker(),
89+
]);
90+
91+
results.forEach(result => {
92+
expect(result.success).toBe(true);
93+
expect(result.error).toBeUndefined();
94+
});
95+
});
96+
5097
test('makes query', async () => {
5198
const testModel: ModelDefinition = {
5299
version: '2.0',

0 commit comments

Comments
 (0)