Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excessive log messages #36

Closed
jhelwig opened this issue Jan 22, 2024 · 3 comments
Closed

Excessive log messages #36

jhelwig opened this issue Jan 22, 2024 · 3 comments

Comments

@jhelwig
Copy link
Contributor

jhelwig commented Jan 22, 2024

Having rfesi log errors results in a lot of unnecessary messages, when I'm handling them in the code that's calling rfesi.

For example: rfesi logs every 404, but intentionally encountering 404s is the only way to make sure that every page of data is retrieved until the response headers (#34) are exposed to end users.

Wrapper to retrieve every page
    #[derive(Debug)]
    pub enum EveClientError {
        Esi(EsiError),
        Other(anyhow::Error),
    }

    impl From<EsiError> for EveClientError {
        fn from(value: EsiError) -> Self {
            Self::Esi(value)
        }
    }

    impl From<anyhow::Error> for EveClientError {
        fn from(value: anyhow::Error) -> Self {
            Self::Other(value)
        }
    }

    impl From<EveClientError> for anyhow::Error {
        fn from(value: EveClientError) -> Self {
            match value {
                EveClientError::Esi(esi_error) => anyhow!(esi_error),
                EveClientError::Other(other) => other,
            }
        }
    }

    pub async fn fetch_all_pages<T, L>(mut fetch_page: L) -> Result<Vec<T>>
    where
        L: AsyncFnMut<(i32,), Output = std::result::Result<Vec<T>,EveClientError>>
    {
        let mut results = Vec::<T>::new();
        let mut current_page = 1;

        loop {
            match fetch_page.call_mut((current_page,)).await {
                Ok(page_results) => results.extend(page_results),
                Err(EveClientError::Esi(EsiError::InvalidStatusCode(404))) => break,
                Err(e) => return Err(e.into()),
            }
            current_page += 1;
        }

        Ok(results)
    }
Example usage
        let mut esi = EsiBuilder::new()
            .user_agent("Your user agent")
            .build()?;
        esi.update_spec().await?;

        let new_market_data = crate::eve_client::fetch_all_pages(
            async_owned_closure_mut!(
                {
                    esi: Esi = esi,
                    region: i32 = region,
                };
                async |page_no: i32| -> std::result::Result<Vec<MarketOrder>, EveClientError> {
                    debug!("Fetching market orders for region {region} page {page_no}.");
                    esi.group_market().get_region_orders(*region, None, Some(page_no), None).await.map_err(Into::into)
                }
            )
        ).await?;

It seems that having rfesi return a Result (as it already does) should be sufficient, and it shouldn't need to also log at the error level internally, especially when it's an error that is able to be handled by the user of the library.

@Celeo
Copy link
Owner

Celeo commented Jan 25, 2024

Yup, fair.

@Celeo
Copy link
Owner

Celeo commented Jan 25, 2024

I've removed the log statement for normal queries, and reduced the level of a few other log statements: 06b270f. Released in 0.39.1.

Let me know if this works better for you.

As a sidenote, I'm learning more just by looking at those snippets you shared, so thanks for that!

@Celeo
Copy link
Owner

Celeo commented Feb 6, 2024

Let me know if there's anything missing from this effort, and I'll happily address. Thanks!

@Celeo Celeo closed this as completed Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants