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

Rework ErrorController now that getErrorPath() is unused and server.error.path must be used to configure the error path #19844

Closed
brucebrit opened this issue Jan 21, 2020 · 19 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@brucebrit
Copy link

brucebrit commented Jan 21, 2020

Spring boot: 2.2.2
IDE: Spring Tool Suite

One of the ways to create a custom error page is to create a controller class that implements ErrorController. By overriding the getErrorPath() method and returning /error, I am able to return the expected my custom error page (located at resources/templates/customError.html).

But when I return another path, other than /error, I get the following error:

This localhost page can’t be found. No webpage was found for the web address: http://localhost:8080/bruce

Why is that so.

@Controller
public class MyErrorController implements ErrorController{

 // @RequestMapping("/error")
    @RequestMapping("/error1")
    public String handleError() {
        return "customError";
    }

	@Override
    public String getErrorPath() {

        //return "/error"; 
        return "/error1"; 
    }
}

Interestingly, I have done another experiment. The getErrorPath() returns /error1 , but I change the RequestMapping annotation to @RequestMapping("/error"), and that really return customError.html. This seems to imply that the path return by getErrorPath() is ignored.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 21, 2020
@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Jan 21, 2020
@philwebb
Copy link
Member

I have a feeling that the interface is unnecessary since Spring Boot 2.0, but I need to double check. I think that setting the server.error.path property is now the only way to customize the actual error path.

If I remember correctly, we used to use ErrorController to allow us to configure security, but we substantially simplified that in Spring Boot 2.0.

@brucebrit
Copy link
Author

brucebrit commented Jan 21, 2020

Hi, thanks for the response.
If that is the case, do we still need to implements ErrorController class, since server.error.path in the properties file is sufficient to achieve what we want.

@philwebb
Copy link
Member

I don't think so, but I've not had time to confirm for sure yet.

@wilkinsona
Copy link
Member

We don't call getErrorPath() anywhere, however implementing ErrorController is required at the moment to cause the auto-configuration of Boot's BasicErrorController to back off:

@Bean
@ConditionalOnMissingBean(value = ErrorController.class, search = SearchStrategy.CURRENT)
public BasicErrorController basicErrorController(ErrorAttributes errorAttributes,
ObjectProvider<ErrorViewResolver> errorViewResolvers) {
return new BasicErrorController(errorAttributes, this.serverProperties.getError(),
errorViewResolvers.orderedStream().collect(Collectors.toList()));
}

I'm not exactly sure what we should do here. An ErrorController marker interface with no methods feels like a bit of a smell, but it does provide a way of identifying the user's error controller and getting the auto-configured one to back off. I can't really think of a better alternative.

@wilkinsona wilkinsona changed the title Custom Error Page not working when getErrorPath() returns path other than "/error" Rework ErrorController now that getErrorPath() is unused and server.error.path must be used to configure the error path Jan 30, 2020
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Jan 30, 2020
@wilkinsona wilkinsona added this to the 2.3.x milestone Jan 30, 2020
@wilkinsona
Copy link
Member

For 2.3.x we could deprecate getErrorPath() and make it a default method so that it doesn't have to be implemented.

@scottfrederick scottfrederick self-assigned this Apr 10, 2020
@scottfrederick
Copy link
Contributor

ErrorController is currently a @FunctionalInterface. We can't add a default implementation for getErrorPath() without removing @FunctionalInterface, which could break backward compatibility if users are creating lambda expressions using ErrorController.

@scottfrederick scottfrederick modified the milestones: 2.3.x, 2.3.0.RC1 Apr 16, 2020
@siller174
Copy link

siller174 commented May 19, 2020

@scottfrederick After 1caca6e we can't build projects with "-Werror" flag, because we must implement getErrorPath(), but method is deprecated

getErrorPath() in org.springframework.boot.web.servlet.error.ErrorController has been deprecated

Compilation failure [ERROR] /.../.../CustomErrorController.java: warnings found and -Werror specified

version 2.3.0.RELEASE

@wilkinsona
Copy link
Member

@siller174 If you want to compile with -Werror you should suppress the deprecation warning using @SuppressWarnings.

@yhojann-cl
Copy link

I have a feeling that the interface is unnecessary since Spring Boot 2.0, but I need to double check. I think that setting the server.error.path property is now the only way to customize the actual error path.

If I remember correctly, we used to use ErrorController to allow us to configure security, but we substantially simplified that in Spring Boot 2.0.

But ErrorController require the deprecated getErrorPath function. When remove says:

CustomErrorController is not abstract and does not override abstract method getErrorPath() in ErrorController

What is the best alternative?

@scottfrederick
Copy link
Contributor

@WHK102 If you provide your own ErrorController implementation, you'll need to implement the getErrorPath method. It can return null or anything you want it to, the return value will be ignored. It's unfortunate, but it's the best option without breaking backward-compatibility of the API.

@xwpongithub
Copy link

@wilkinsona According to this configuration, how to return json instead of a template path when a 404 error occurs

@wilkinsona
Copy link
Member

@xwpongithub Sorry, I don't see the connection between this issue and your question. Also, as mentioned in the guidelines for contributing, we prefer to use GitHub issues only for bugs and enhancements. Please use Stack Overflow or Gitter for questions like this.

@nkavian
Copy link

nkavian commented Apr 5, 2021

@wilkinsona I'm saddened by this. The maven-compiler-plugin keeps reporting the deprecation with and without SuppressWarnings, so I can't get it to stop. Eclipse doesn't complain, just maven. I take pride in having clean code and this sucks as my only unresolvable item. Using Spring Boot 2.4.4. when will this deprecation be fully removed?

@scottfrederick
Copy link
Contributor

@nkavian Per the project policy, the method that was deprecated in 2.3.0 has been removed for the upcoming 2.5.0 release.

@wilkinsona
Copy link
Member

@nkavian I'm not sure why the compiler that Maven's using (typically javac) wouldn't respect the suppression of deprecation warnings. Until you've upgraded to Boot 2.5, you may also want to try marking your own implementation as @Deprecated.

@nkavian
Copy link

nkavian commented Apr 6, 2021

Wow, that trick worked! Thanks.

de-jcup added a commit to mercedes-benz/sechub that referenced this issue Jul 2, 2021
- Former deprecated method ErrorController#getErrorPath was removed
  in Spring Boot 2.5.
  see spring-projects/spring-boot#19844
- setting now `server.error.path` in PDS and SecHub server and using
  the variable inside error controller implementations
de-jcup added a commit to mercedes-benz/sechub that referenced this issue Jul 6, 2021
- upgraded used Spring Boot version to 2.5.2
- upgraded gradle wrapper and gradle version to 6.9 (necessary for newer Spring Boot version)
- fixed compile errors with spring boot 2.5:
  - former deprecated method ErrorController#getErrorPath was removed in Spring Boot 2.5.
    see spring-projects/spring-boot#19844
  - setting now `server.error.path` in PDS and SecHub server and using the variable inside error controller
    implementations
- fixed failing wiremock tests by changing multipart checks (because now always with boundary)
j-sandy added a commit to j-sandy/kork that referenced this issue Oct 18, 2022
….5.14

While upgrading the spring-boot, the compilation of kork-web module failed with following error:
```
> Task :kork-web:compileGroovy FAILED
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:41: error: incompatible types: Boolean cannot be converted to ErrorAttributeOptions
        errorAttributes.getErrorAttributes(webRequest, includeStackTrace);
                                                       ^
```

```
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:51: error: method does not override or implement a method from a supertype
  @OverRide
  ^
```

```
> Task :kork-web:compileGroovy
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/selector/v2/SelectableService.java uses unchecked or unsafe operations.
startup failed:
/kork/kork-web/src/main/groovy/com/netflix/spinnaker/config/ErrorConfiguration.groovy: 37: Method 'getErrorAttributes' from class 'com.netflix.spinnaker.config.ErrorConfiguration$1' does not override method from its superclass or interfaces but is annotated with @OverRide.
 @ line 37, column 7.
         @OverRide
         ^
```
The root cause is the deprecation of following methods in Spring boot 2.3.x and now removal of code from spring boot 2.5.x:

ErrorAttributes.getErrorAttributes(ServerRequest, boolean)
spring-projects/spring-boot@158933c    spring-projects/spring-boot#21324

ErrorController.getErrorPath()
spring-projects/spring-boot#19844

Fixed the issue with required code changes.
j-sandy added a commit to j-sandy/kork that referenced this issue Mar 30, 2023
….5.14

While upgrading the spring-boot, the compilation of kork-web module failed with following error:
```
> Task :kork-web:compileGroovy FAILED
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:41: error: incompatible types: Boolean cannot be converted to ErrorAttributeOptions
        errorAttributes.getErrorAttributes(webRequest, includeStackTrace);
                                                       ^
```

```
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:51: error: method does not override or implement a method from a supertype
  @OverRide
  ^
```

```
> Task :kork-web:compileGroovy
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/selector/v2/SelectableService.java uses unchecked or unsafe operations.
startup failed:
/kork/kork-web/src/main/groovy/com/netflix/spinnaker/config/ErrorConfiguration.groovy: 37: Method 'getErrorAttributes' from class 'com.netflix.spinnaker.config.ErrorConfiguration$1' does not override method from its superclass or interfaces but is annotated with @OverRide.
 @ line 37, column 7.
         @OverRide
         ^
```
The root cause is the deprecation of following methods in Spring boot 2.3.x and now removal of code from spring boot 2.5.x:

ErrorAttributes.getErrorAttributes(ServerRequest, boolean)
spring-projects/spring-boot@158933c    spring-projects/spring-boot#21324

ErrorController.getErrorPath()
spring-projects/spring-boot#19844

Fixed the issue with required code changes.
j-sandy added a commit to j-sandy/kork that referenced this issue Mar 31, 2023
….5.14

While upgrading the spring-boot, the compilation of kork-web module failed with following error:
```
> Task :kork-web:compileGroovy FAILED
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:41: error: incompatible types: Boolean cannot be converted to ErrorAttributeOptions
        errorAttributes.getErrorAttributes(webRequest, includeStackTrace);
                                                       ^
```

```
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:51: error: method does not override or implement a method from a supertype
  @OverRide
  ^
```

```
> Task :kork-web:compileGroovy
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/selector/v2/SelectableService.java uses unchecked or unsafe operations.
startup failed:
/kork/kork-web/src/main/groovy/com/netflix/spinnaker/config/ErrorConfiguration.groovy: 37: Method 'getErrorAttributes' from class 'com.netflix.spinnaker.config.ErrorConfiguration$1' does not override method from its superclass or interfaces but is annotated with @OverRide.
 @ line 37, column 7.
         @OverRide
         ^
```
The root cause is the deprecation of following methods in Spring boot 2.3.x and now removal of code from spring boot 2.5.x:

ErrorAttributes.getErrorAttributes(ServerRequest, boolean)
spring-projects/spring-boot@158933c    spring-projects/spring-boot#21324

ErrorController.getErrorPath()
spring-projects/spring-boot#19844

Fixed the issue with required code changes.
j-sandy added a commit to j-sandy/kork that referenced this issue Mar 31, 2023
….5.14

While upgrading the spring-boot, the compilation of kork-web module failed with following error:
```
> Task :kork-web:compileGroovy FAILED
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:41: error: incompatible types: Boolean cannot be converted to ErrorAttributeOptions
        errorAttributes.getErrorAttributes(webRequest, includeStackTrace);
                                                       ^
```

```
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:51: error: method does not override or implement a method from a supertype
  @OverRide
  ^
```

```
> Task :kork-web:compileGroovy
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/selector/v2/SelectableService.java uses unchecked or unsafe operations.
startup failed:
/kork/kork-web/src/main/groovy/com/netflix/spinnaker/config/ErrorConfiguration.groovy: 37: Method 'getErrorAttributes' from class 'com.netflix.spinnaker.config.ErrorConfiguration$1' does not override method from its superclass or interfaces but is annotated with @OverRide.
 @ line 37, column 7.
         @OverRide
         ^
```
The root cause is the deprecation of following methods in Spring boot 2.3.x and now removal of code from spring boot 2.5.x:

ErrorAttributes.getErrorAttributes(ServerRequest, boolean)
spring-projects/spring-boot@158933c    spring-projects/spring-boot#21324

ErrorController.getErrorPath()
spring-projects/spring-boot#19844

Fixed the issue with required code changes.
mergify bot pushed a commit to spinnaker/kork that referenced this issue Mar 31, 2023
* chore(dependencies): Upgrade Spring Boot to 2.5.14

* fix(dependency): Issue with kork-jedis while upgrading spring-boot to 2.5.14

While upgrading the spring-boot, the compilation of kork-jedis module failed with following error:
```
> Task :kork-jedis:compileJava FAILED
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2652: error: slowlogGetBinary(long) in InstrumentedJedis cannot implement slowlogGetBinary(long) in AdvancedBinaryJedisCommands
  public List<byte[]> slowlogGetBinary(long entries) {
                      ^
  return type List<byte[]> is not compatible with List<Object>
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2646: error: slowlogGetBinary() in InstrumentedJedis cannot implement slowlogGetBinary() in AdvancedBinaryJedisCommands
  public List<byte[]> slowlogGetBinary() {
                      ^
  return type List<byte[]> is not compatible with List<Object>
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2645: error: method does not override or implement a method from a supertype
  @OverRide
  ^
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2648: error: incompatible types: inference variable T has incompatible bounds
    return instrumented(command, () -> delegated.slowlogGetBinary());
                       ^
    lower bounds: List<byte[]>,Object
    lower bounds: List<Object>
  where T is a type-variable:
    T extends Object declared in method <T>instrumented(String,Callable<T>)
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2651: error: method does not override or implement a method from a supertype
  @OverRide
  ^
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2654: error: incompatible types: inference variable T has incompatible bounds
    return instrumented(command, () -> delegated.slowlogGetBinary(entries));
                       ^
    lower bounds: List<byte[]>,Object
    lower bounds: List<Object>
  where T is a type-variable:
    T extends Object declared in method <T>instrumented(String,Callable<T>)
```

```
> Task :kork-jedis:compileJava FAILED
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedisPool.java:84: error: name clash: initPool(GenericObjectPoolConfig,PooledObjectFactory<Jedis>) in InstrumentedJedisPool and initPool(GenericObjectPoolConfig<Jedis>,PooledObjectFactory<Jedis>) in Pool have the same erasure, yet neither overrides the other
  public void initPool(GenericObjectPoolConfig poolConfig, PooledObjectFactory<Jedis> factory) {
              ^
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedisPool.java:83: error: method does not override or implement a method from a supertype
  @OverRide
```

The root cause is the upgrade of redis.clients:jedis from 3.3.0 to 3.6.3 as transitive dependency of spring-boot, that brings the breaking changes in APIs as mentioned below:
redis/jedis#2084
redis/jedis#2361

Fixed the issue with required code changes.

* fix(dependency): Issue with kork-web while upgrading spring-boot to 2.5.14

While upgrading the spring-boot, the compilation of kork-web module failed with following error:
```
> Task :kork-web:compileGroovy FAILED
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:41: error: incompatible types: Boolean cannot be converted to ErrorAttributeOptions
        errorAttributes.getErrorAttributes(webRequest, includeStackTrace);
                                                       ^
```

```
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:51: error: method does not override or implement a method from a supertype
  @OverRide
  ^
```

```
> Task :kork-web:compileGroovy
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/selector/v2/SelectableService.java uses unchecked or unsafe operations.
startup failed:
/kork/kork-web/src/main/groovy/com/netflix/spinnaker/config/ErrorConfiguration.groovy: 37: Method 'getErrorAttributes' from class 'com.netflix.spinnaker.config.ErrorConfiguration$1' does not override method from its superclass or interfaces but is annotated with @OverRide.
 @ line 37, column 7.
         @OverRide
         ^
```
The root cause is the deprecation of following methods in Spring boot 2.3.x and now removal of code from spring boot 2.5.x:

ErrorAttributes.getErrorAttributes(ServerRequest, boolean)
spring-projects/spring-boot@158933c    spring-projects/spring-boot#21324

ErrorController.getErrorPath()
spring-projects/spring-boot#19844

Fixed the issue with required code changes.

* chore (dependency): pinning the io.rest-assured to 4.2.0

Spring boot 2.5.14 upgrade brings io.rest-assured 4.3.3 as transitive dependency.
io.rest-assured 4.3.x require groovy 3.0.2. So, pinning the nearest version using groovy 2.x. After upgrading the groovy to 3.x, pin can be removed. [https://github.com/rest-assured/rest-assured/blob/9b683130c93188cabdef850e89d0c9417d847a17/changelog.txt#L200]

* chore(dependency): pinning ch.qos.logback to 1.2.10

Spring boot 2.5.14 upgrade brings ch.qos.logback 1.2.11 as transitive dependency.
A bug is reported in 1.2.11 [https://jira.qos.ch/browse/LOGBACK-1623] and it is fixed in 1.2.12.
However the 1.2.12 package has not been released yet. So, pinning the version to 1.2.10 untill required package is released.

* chore(dependency): Pinning groovy to 2.5.15 with spring boot 2.5.x upgrade

Spring boot 2.5.x brings groovy 3.x as its transitive dependency.
https://docs.spring.io/spring-boot/docs/2.5.14/reference/html/dependency-versions.html#appendix.dependency-versions

Currently spinnaker services use gradle 6.x, that does not support groovy 3.x.
https://docs.gradle.org/6.8.1/userguide/compatibility.html

Restricting groovy to 2.5.x, till upgrade of gradle to 7.x.
https://docs.gradle.org/current/userguide/resolution_rules.html#sec:denying_version
To avoid transitive upgrade of groovy, pinning it with enforcedPlatform() closure.
It forces version for internal submodules of kork as well as for all the consumer spinnaker services.

* chore(dependency): pin org.liquibase to 3.10.3

 While upgrading spring boot 2.5.x, liquibase version transitively upgrades to 4.3.5. The liquibase version starting from 4.0.0 till 4.12.0 has an [issue](liquibase/liquibase#2818) w.r.t parsing the changelog file, if found at multiple places within the classpath and encounter the below error:
 ```
 Caused by: liquibase.exception.ChangeLogParseException: Error parsing classpath:db/healthcheck.yml
	at liquibase.parser.core.yaml.YamlChangeLogParser.parse(YamlChangeLogParser.java:89)
	at liquibase.Liquibase.getDatabaseChangeLog(Liquibase.java:369)
	at liquibase.Liquibase.lambda$update$1(Liquibase.java:224)
	at liquibase.Scope.lambda$child$0(Scope.java:180)
	at liquibase.Scope.child(Scope.java:189)
	at liquibase.Scope.child(Scope.java:179)
	at liquibase.Scope.child(Scope.java:158)
	at liquibase.Liquibase.runInScope(Liquibase.java:2405)
	at liquibase.Liquibase.update(Liquibase.java:211)
	at liquibase.Liquibase.update(Liquibase.java:197)
	at liquibase.integration.spring.SpringLiquibase.performUpdate(SpringLiquibase.java:314)
	at liquibase.integration.spring.SpringLiquibase.afterPropertiesSet(SpringLiquibase.java:269)
	at com.netflix.spinnaker.kork.sql.migration.SpringLiquibaseProxy.afterPropertiesSet(SpringLiquibaseProxy.kt:65)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1858)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1795)
	... 87 more
Caused by: java.io.IOException: Found 2 files that match classpath:db/healthcheck.yml: file:/spinnaker/kork/kork-sql/build/resources/main/db/healthcheck.yml, jar:file:/spinnaker/kork/kork-sql/build/libs/kork-sql.jar!/db/healthcheck.yml
	at liquibase.resource.AbstractResourceAccessor.openStream(AbstractResourceAccessor.java:25)
	at liquibase.parser.core.yaml.YamlChangeLogParser.parse(YamlChangeLogParser.java:25)
	... 101 more
 ```
 This duplicate changelog issue is fixed in 4.13.0, but identified another issue that gets introduced in 4.13.0. This [issue](liquibase/liquibase#3091) hinders the migration of sql scripts available in [orca](https://github.com/spinnaker/orca/tree/master/orca-sql/src/main/resources/db/changelog), containing `afterColumn`, with a validation error for postgresql.
 The efforts to resolve the issue are in progress, so pinning the version of org.liquibase:liquibase-core to 3.10.3 (latest of 3.x series).
@brsolomon-deloitte
Copy link

@nkavian Per the project policy, the method that was deprecated in 2.3.0 has been removed for the upcoming 2.5.0 release.

No mention of this in 2.3 release notes.

https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.3-Release-Notes

@wilkinsona
Copy link
Member

Thanks, @brsolomon-deloitte. I've updated the release notes to list the deprecation.

@brsolomon-deloitte
Copy link

Thanks for doing that @wilkinsona . I have been tasked with bringing an old Java lib kicking and screaming into 2023, so I've learned my lesson here to compile strict and turn deprecation warnings into errors to begin with :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

10 participants