This should just throw IOException

This is used in renderTeamMember and next in renderBody method.

• renderBody method is implementation of org.apache.maven.reporting.AbstractMavenReportRenderer
• renderBody can not throw any exception

so when exception should be catch and what we can do with it ...?

This doesn't handle the exception, just pretends it's not going to happen. Checked or unchecked it needs to be handled. I don't think it can be handled here, so throw the IOException and handle a failure where it's called from.

ok, what should happen on such exception, how do you want to handle it in report?

Now it be handled by Maven itself and build will be break.
I hope it will not often exception.

If breaking the build is the right answer — though I'm not sure it is — then that should happen higher up, likely with a MavenReportException or something like that. It shouldn't be a runtime exception from deep inside the code, whihc would indicate a bug in the code. An avatar service being unreachable is not a bug in the code, so just use a regular IOException here.

from team report perspective fetching images can be interpreted as one of report feature - so it can be important

renderBody method is defined in AbstractMavenReportRenderer from maven-reporting-impl so can not be changed in this PR.

That doesn't sound right. This is a private method so something in this class should be calling it.

There is use, line 279:

         picUrl = getExternalAvatarUrl(member.getEmail());

so handle the IOException there, not here.

Also, if renderBody cannot throw any exception, then don't throw an exception in renderBody. Make sure that whatever code you call inside renderBody does not throw any exception, runtime exceptions included. Instead call all the code that might throw exceptions before you ever call renderBody, load up all the URLs or pics or whatever in memory at the beginning of executeReport, and only start rendering once all the resources are loaded and will not throw any exceptions during rendering.

Though in this case I really think it's simpler and easier to just use the default spacer image.

URL.
Is there an internal URL? If not, delete "external" here or rewrite

I've rewritten a little here ...

This doesn't handle the exception, just pretends it's not going to happen. Checked or unchecked it needs to be handled. I don't think it can be handled here, so throw the IOException and handle a failure where it's called from.

What do you want to do here when getExternalAvatarUrl( fails and throws an exception? Probably just don't call sink.figureGraphics below.

I want break a build, because images in report can be missing.

What if a user doesn't have an avatar? I don't think I do, for instance.

when user doesn't have an avatar default one will be returned by gravatar

Looks like you can handle the IOException in this method. It doesn't need tonescape this class. Just wrap lines 277 through 285 in a try-catch block. The catch block can be empty or it can fill in a default image.

I know that I can catch and skip exception, and use default images - but I don't want.

As you know one step of release new component is publishing new documentation ... I don't want to add next manual task to verify team page during release. Maybe you have another experience with releasing new components ...

Some context of change - look at our team pages: https://maven.apache.org/team.html the same are on many other ASF projects.

Would it make sense to instead simply not show avatars in the report?

user want to show avatars, and probably misconfigured plugin - so we should not hide such case

I suggest message then, and not breaking the report. You could also add a default image used when the server can't be reached.

It is two different case wrong configuration - not existing provider and not available image server.

In both case we should break a build to prevent of unwanted result

I suggest message then, and not breaking the report. You could also add a default image used when the server can't be reached.

What if a user doesn't have an avatar? I don't think I do, for instance.

If breaking the build is the right answer — though I'm not sure it is — then that should happen higher up, likely with a MavenReportException or something like that. It shouldn't be a runtime exception from deep inside the code, whihc would indicate a bug in the code. An avatar service being unreachable is not a bug in the code, so just use a regular IOException here.

Not fetching an artifact is far more serious than not fetching a user pic. It's reasonable that one breaks the build and the other doesn't.

If you can't throw MavenReportException from renderBody then pass the IOException up the stack to something that can. Or change the method signature so it can throw, though here I really think the exception can simply be handled by dropping the image. Perhaps this exception can be handled by using the default image.

That doesn't sound right. This is a private method so something in this class should be calling it.

Looks like you can handle the IOException in this method. It doesn't need tonescape this class. Just wrap lines 277 through 285 in a try-catch block. The catch block can be empty or it can fill in a default image.

You can probably make this non-private and use this in cases where the provider fails.

so handle the IOException there, not here.

Also, if renderBody cannot throw any exception, then don't throw an exception in renderBody. Make sure that whatever code you call inside renderBody does not throw any exception, runtime exceptions included. Instead call all the code that might throw exceptions before you ever call renderBody, load up all the URLs or pics or whatever in memory at the beginning of executeReport, and only start rendering once all the resources are loaded and will not throw any exceptions during rendering.

Though in this case I really think it's simpler and easier to just use the default spacer image.