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

spring.mvc.log-resolved-exception gets enabled by default. #29706

Closed
cdprete opened this issue Feb 9, 2022 · 17 comments
Closed

spring.mvc.log-resolved-exception gets enabled by default. #29706

cdprete opened this issue Feb 9, 2022 · 17 comments
Labels
status: invalid An issue that we don't feel is valid

Comments

@cdprete
Copy link

cdprete commented Feb 9, 2022

Hi.
I'm using Spring Boot 2.6.3 and from the moment I've defined a ResponseEntityExceptionHandler I get an annoying warning log in the console of the type WARN 7072 --- [nio-8080-exec-2] .m.m.a.ExceptionHandlerExceptionResolver : Resolved [<fully-qualified-class-name>: <message>].

The handler is the following:

/**
 * @author Cosimo Damiano Prete
 * @since 09/02/2022
 */
@ControllerAdvice
class WebServiceExceptionHandler extends ResponseEntityExceptionHandler {

    @Valid
    @ExceptionHandler(ClientHttpException.class)
    public ResponseEntity<Object> handleClientHttpException(ClientHttpException ex, WebRequest request) {
        return handleExceptionInternal(ex, ErrorResponse.fromClientHttpException(ex), new HttpHeaders(), ex.statusCode(), request);
    }

    private record ErrorResponse(String code, String defaultMessage) {
        private static <E extends ClientHttpException> ErrorResponse fromClientHttpException(E ex) {
            return new ErrorResponse(ex.clientCode(), ex.getMessage());
        }
    }
}

By putting a breakpoint in the setter of WebMvcProperties I can see it actually gets called with the value true even though I don't have such value set at all in my application.yml file.
Strangely enough, if I explicitly set it to false then it stays like that (probably my props are applied later).

The issue doesn't seem to be caused by the ResponseEntityExceptionHandler, since I get the same also when the handler is defined as

/**
 * @author Cosimo Damiano Prete
 * @since 09/02/2022
 */
@ControllerAdvice
class WebServiceExceptionHandler {

    @ExceptionHandler(ClientHttpException.class)
    ResponseEntity<ErrorResponse> handleClientHttpException(ClientHttpException ex) {
        return new ResponseEntity<>(ErrorResponse.fromClientHttpException(ex), ex.statusCode());
    }

    private record ErrorResponse(String code, String defaultMessage) {
        private static <E extends ClientHttpException> ErrorResponse fromClientHttpException(E ex) {
            return new ErrorResponse(ex.clientCode(), ex.getMessage());
        }
    }
}

I've also tried adding an @Order annotation with highest priority (so that it would trigger my advice first), but it didn't help.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 9, 2022
@scottfrederick
Copy link
Contributor

scottfrederick commented Feb 9, 2022

Thanks for the report. I haven't been able to reproduce the resolver handler logging by default with a trivial sample using the code snippets you've provided. It's possible there is other configuration in your application that isn't shown here. If you would like us to spend more time investigating, please provide a complete minimal sample that reproduces the problem. You can share it with us by pushing it to a separate repository on GitHub or by zipping it and attaching it to this issue.

@scottfrederick scottfrederick added the status: waiting-for-feedback We need additional information before we can continue label Feb 9, 2022
@cdprete
Copy link
Author

cdprete commented Feb 9, 2022

Hi. My entire configuration is composed by the following 3 files:

application.yaml

spring:
  liquibase.change-log: db/changelog/db.changelog-master.xml
  jpa:
    open-in-view: false
  data.web.pageable:
    one-indexed-parameters: true
    max-page-size: 500
  jackson.default-property-inclusion: non_null
server:
  shutdown: graceful
  servlet:
    jsp.registered: false
    context-path: /p-b

application-dev.yaml (active)

spring.datasource:
  driver-class-name: org.h2.Driver
  url: jdbc:h2:mem:pb-db;DB_CLOSE_ON_EXIT=FALSE

spring.properties

spring.jndi.ignore=true

Also, I'm not setting anything programmatically apart

/**
 * @author Cosimo Damiano Prete
 * @since 31/01/2022
 */
@Configuration
@EnableTransactionManagement
@EnableJpaAuditing(
        auditorAwareRef = PersistenceConfig.AUDITOR_AWARE_BEAN_NAME,
        dateTimeProviderRef = PersistenceConfig.DATE_TIME_PROVIDER_BEAN_NAME
)
public class PersistenceConfig {
    static final String AUDITOR_AWARE_BEAN_NAME = "auditorAware";
    static final String DATE_TIME_PROVIDER_BEAN_NAME = "dateTimeProvider";

    @Bean(name = AUDITOR_AWARE_BEAN_NAME)
    AuditorAware<String> auditorAware() {
        return () -> Optional.of("tbd");
    }

    @Bean(name = DATE_TIME_PROVIDER_BEAN_NAME)
    DateTimeProvider dateTimeProvider() {
        return () -> Optional.of(OffsetDateTime.now(Clock.systemUTC()));
    }
}

and my main class is just

@SpringBootApplication
public class PBApplication {
    public static void main(String[] args) {
        SpringApplication.run(PBApplication.class, args);
    }
}

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 9, 2022
@scottfrederick
Copy link
Contributor

@cdprete I still l can't reproduce the issue with the information provided. There are additional libraries in your application and possibly other code that isn't shown, and it's not a good use of our time to try to re-create everything you are doing. As requested, please provide a sample demonstrating the problem that we can compile and run ourselves.

@scottfrederick scottfrederick added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 9, 2022
@wilkinsona
Copy link
Member

wilkinsona commented Feb 10, 2022

@cdprete Are you using DevTools? It enables logging of resolved exceptions by default:

The complete sample that Scott has requested a couple of times would have answered this definitively.

@cdprete
Copy link
Author

cdprete commented Feb 10, 2022

Hi @wilkinsona.
Yes, I do have DevTools enabled.
I'll test this later and notify you if removing the dev tools resolves the issue. If it doesn't solve the issue, I'll try to anonymize the code so that I can post it as example.

That said, I think this should properly be documented ;)

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 10, 2022
@wilkinsona
Copy link
Member

There's already a section in the reference docs about DevTools' property defaults, including a link to the class to which I linked above. #29406 is tracking inlining the default properties in the documentation in place of the link to DevToolsPropertyDefaultsPostProcessor.

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 10, 2022
@cdprete
Copy link
Author

cdprete commented Feb 10, 2022

HI.
I've sent an invitation both of you @scottfrederick and @wilkinsona for accessing the repository :)
Even if I remove the dependency to the devtools, the issue remains.
Moreover, the DB_CLOSE_ON_EXIT=FALSE seems to be ignored, but maybe they have different causes...

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 10, 2022
@wilkinsona
Copy link
Member

wilkinsona commented Feb 11, 2022

Thanks for the access to the repository. Unfortunately, it doesn't reproduce the behaviour that you have described. With the dependency on DevTools in place, I can see that org.springframework.boot.autoconfigure.web.servlet.WebMvcProperties.setLogResolvedException(boolean) is called with true as expected. When I remove the dependency on DevTools, the method is no longer called and the default of false is used.

If you'd like us to spend any more time on this, we'll need a sample that reproduces the problem that you have described. It should also be as minimal as possible to reproduce your specific problem. It took me some time to get your sample to run as I had to, for example, update the DB configuration. I'm afraid I won't have time to look at such an unnecessarily complex sample next time.

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 11, 2022
@cdprete
Copy link
Author

cdprete commented Feb 11, 2022

Hi.
How did you exactly remove the dependency to the dev tools? Just from the pom.xml?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 11, 2022
@wilkinsona
Copy link
Member

Yes. I removed it from the pom.xml, double-checked that my IDE had updated the classpath to remove the dependency, and then ran the application again.

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 11, 2022
@cdprete
Copy link
Author

cdprete commented Feb 11, 2022

That's very strange...
I created a new project to test this and I cannot reproduce the behaviour. I think I need to check all the dependencies or so.

For what concern the DB, the warning is

2022-02-11 18:45:46.144  WARN 14476 --- [ionShutdownHook] o.s.b.f.support.DisposableBeanAdapter    : Invocation of destroy method failed on bean with name 'inMemoryDatabaseShutdownExecutor': org.h2.jdbc.JdbcSQLNonTransientConnectionException: Database is already closed (to disable automatic closing at VM shutdown, add ";DB_CLOSE_ON_EXIT=FALSE" to the db URL) [90121-200]

but it happens only with the devtools enabled.

@spring-projects-issues spring-projects-issues added the status: feedback-provided Feedback has been provided label Feb 11, 2022
@spring-projects-issues spring-projects-issues removed the status: waiting-for-feedback We need additional information before we can continue label Feb 11, 2022
@wilkinsona
Copy link
Member

DevTools automatically shuts down the database on each restart (close of the context) to ensure that it can then be recreated and any schema.sql and data.sql scripts can be applied. In other words, the warning you are seeing is to be expected (and is benign) given your configuration and use of DevTools.

@wilkinsona wilkinsona added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Feb 11, 2022
@cdprete
Copy link
Author

cdprete commented Feb 11, 2022

In my "example", I was able to narrow down the issue further.
The infamous log happens only on the form-data multipart request.

org.springframework.web.servlet.handler.AbstractHandlerExceptionResolver#resolveException gets called 3 times and on the third time doResolveException returns an empty but not null ModelAndView.
Then, org.springframework.web.servlet.handler.AbstractHandlerExceptionResolver#logException on line 149 is called which does the logging.

@wilkinsona
Copy link
Member

If you can provide a minimal sample that reproduces that behaviour, we can take another look. It should be something that's as small as possible to reproduce the problem and include a test or some simple instructions that show how to do so.

cdprete pushed a commit to cdprete/github_spring_boot_issue_29706 that referenced this issue Feb 11, 2022
cdprete pushed a commit to cdprete/github_spring_boot_issue_29706 that referenced this issue Feb 11, 2022
cdprete pushed a commit to cdprete/github_spring_boot_issue_29706 that referenced this issue Feb 11, 2022
@cdprete
Copy link
Author

cdprete commented Feb 11, 2022

@philwebb philwebb reopened this Feb 11, 2022
@philwebb philwebb added status: waiting-for-triage An issue we've not yet triaged and removed status: invalid An issue that we don't feel is valid labels Feb 11, 2022
@wilkinsona
Copy link
Member

Thanks for the minimal sample. Unlike in your original description, the exception is being logged by DefaultHandlerExceptionResolver:

2022-02-11 20:47:35.281  WARN 81983 --- [nio-8080-exec-1] .w.s.m.s.DefaultHandlerExceptionResolver : Resolved [org.springframework.http.converter.HttpMessageNotReadableException: JSON parse error: Cannot coerce empty String ("") to `com.cdprete.demo.DemoDto$DemoEnum` value (but could if coercion was enabled using `CoercionConfig`); nested exception is com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot coerce empty String ("") to `com.cdprete.demo.DemoDto$DemoEnum` value (but could if coercion was enabled using `CoercionConfig`)<EOL> at [Source: (PushbackInputStream); line: 1, column: 23] (through reference chain: com.cdprete.demo.DemoDto["values"]->java.util.ArrayList[0]->com.cdprete.demo.DemoDto$DemoInnerDto["value"])]

log-resolved-exception property is working as documented:

Whether to enable warn logging of exceptions resolved by a "HandlerExceptionResolver", except for "DefaultHandlerExceptionResolver".

See #14936 for some additional details.

You may want to add an @ExceptionHandler for HttpMessageNotReadableException to customize its exception handling and avoid this logging.

@wilkinsona wilkinsona added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 11, 2022
@cdprete
Copy link
Author

cdprete commented Feb 11, 2022

Oh, that's true. The handler is different.
Then it had to be the DevTools :)

Thanks for the feedbacks ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

5 participants