Skip to content
This repository has been archived by the owner on Jul 26, 2024. It is now read-only.

bug: report error from image library #349

Merged
merged 9 commits into from
Feb 4, 2022
Merged

bug: report error from image library #349

merged 9 commits into from
Feb 4, 2022

Conversation

jrconlin
Copy link
Member

Closes: #348

@jrconlin jrconlin requested a review from a team January 25, 2022 22:46
@ncloudioj
Copy link
Collaborator

@jrconlin When encountering this kind of error, is it possible for Contile to bail out rather than return an error code in the response? Doing so at least allows us to return some good tiles instead of nothing?

@jrconlin
Copy link
Member Author

jrconlin commented Feb 3, 2022

This is currently failing tests because of the test suite change for mobile endpoints. #346
Rather than spend time correcting for this change, I'm going to wait until after the multi-endpoint change lands, then fix things here.

.map_err(|_| HandlerErrorKind::BadImage("Image unreadable"))?;
let img = reader.decode().map_err(|e| {
let mut tags = Tags::default();
tags.add_extra("error", &e.to_string());
Copy link
Member

@pjenvey pjenvey Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think both url and format are really worth adding as extras here too -- the easiest way to do that is to probably shuffle this method around a bit to have the same signature as validate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh.
the change was included in the merge e767a61

@jrconlin jrconlin requested a review from pjenvey February 4, 2022 17:28
src/adm/tiles.rs Outdated
@@ -212,6 +212,7 @@ pub async fn get_tiles(
// We still want to track this as a server error later.
//
// TODO: Remove this after the shared cache is implemented.
dbg!(&e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgot to kill this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argh... hang on.

@jrconlin jrconlin merged commit 8d13e88 into main Feb 4, 2022
@jrconlin jrconlin deleted the bug/348-img branch February 4, 2022 20:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report error from image reader
3 participants