Skip to content

[bugfix] fixed "jackson enum conversion : InvalidFormatException"#3078

Merged
davidzollo merged 5 commits intoapache:devfrom
muzhongjiang:dev
Jul 2, 2020
Merged

[bugfix] fixed "jackson enum conversion : InvalidFormatException"#3078
davidzollo merged 5 commits intoapache:devfrom
muzhongjiang:dev

Conversation

@muzhongjiang
Copy link
Copy Markdown
Contributor

fixed bug "jackson enum conversion : InvalidFormatException"

[ERROR] 2020-06-29 20:02:08.580 org.apache.dolphinscheduler.common.utils.JSONUtils:[109] - parse object exception!
com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type org.apache.dolphinscheduler.common.enums.TaskTimeoutStrategy from String "": value not one of declared Enum instance names: [WARNFAILED, FAILED, WARN]
at [Source: (String)"{"strategy":"","interval":null,"enable":false}"; line: 1, column: 13] (through reference chain: org.apache.dolphinscheduler.common.task.TaskTimeoutParameter["strategy"])
at com.fasterxml.jackson.databind.exc.InvalidFormatException.from(InvalidFormatException.java:67)
at com.fasterxml.jackson.databind.DeserializationContext.weirdStringException(DeserializationContext.java:1549)
at com.fasterxml.jackson.databind.DeserializationContext.handleWeirdStringValue(DeserializationContext.java:911)
at com.fasterxml.jackson.databind.deser.std.EnumDeserializer._deserializeAltString(EnumDeserializer.java:255)
at com.fasterxml.jackson.databind.deser.std.EnumDeserializer.deserialize(EnumDeserializer.java:179)
at com.fasterxml.jackson.databind.deser.impl.MethodProperty.deserializeAndSet(MethodProperty.java:127)
at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:288)
at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:151)
at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4013)
at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3004)
at org.apache.dolphinscheduler.common.utils.JSONUtils.parseObject(JSONUtils.java:107)

Verify this pull request

image

@yangyichao-mango
Copy link
Copy Markdown
Contributor

Hi,
You can check this same error issue [1], and which branch of you error.
In my opinion, if this error is actually happened in program, we should fix it in front-end but not back-end Jackson serde function, because it should throw exception when deserialize from an empty string enum json string.

你可以看下 [1] 中的issue,也是同样的问题。你的分支是哪个分支呢?
个人理解,如果这个错误确实存在,我理解我们最好应该在前端向后端传参时就避免这个问题,如果后端在序列化上去处理这个问题的话,可能会导致吞掉一些其他枚举类型在序列化时本应该抛出来的错误。

[1] #3028

@muzhongjiang
Copy link
Copy Markdown
Contributor Author

Hi,
You can check this same error issue [1], and which branch of you error.
In my opinion, if this error is actually happened in program, we should fix it in front-end but not back-end Jackson serde function, because it should throw exception when deserialize from an empty string enum json string.

你可以看下 [1] 中的issue,也是同样的问题。你的分支是哪个分支呢?
个人理解,如果这个错误确实存在,我理解我们最好应该在前端向后端传参时就避免这个问题,如果后端在序列化上去处理这个问题的话,可能会导致吞掉一些其他枚举类型在序列化时本应该抛出来的错误。

[1] #3028

After testing 1.3.0 Release version does not have this problem, dev has this problem.
This is because the fastjson used in 1.3.0 Release is adapted to the deserialization of enum.
Now replacing it with jackson should ensure that the function is consistent with the Release version, so the configuration item READ_UNKNOWN_ENUM_VALUES_AS_NULL is opened

经过测试 1.3.0 Release 版本没有这个问题,dev 有这个问题。
是因为 1.3.0 Release 用的 fastjson,fastjson对 enum 反序列化做了适配。
现在替换为 jackson 应该保证功能和Release版本一致,所以打开了这个配置项READ_UNKNOWN_ENUM_VALUES_AS_NULL

@yangyichao-mango
Copy link
Copy Markdown
Contributor

yangyichao-mango commented Jun 30, 2020

Hi,
You can check this same error issue [1], and which branch of you error.
In my opinion, if this error is actually happened in program, we should fix it in front-end but not back-end Jackson serde function, because it should throw exception when deserialize from an empty string enum json string.
你可以看下 [1] 中的issue,也是同样的问题。你的分支是哪个分支呢?
个人理解,如果这个错误确实存在,我理解我们最好应该在前端向后端传参时就避免这个问题,如果后端在序列化上去处理这个问题的话,可能会导致吞掉一些其他枚举类型在序列化时本应该抛出来的错误。
[1] #3028

After testing 1.3.0 Release version does not have this problem, dev has this problem.
This is because the fastjson used in 1.3.0 Release is adapted to the deserialization of enum.
Now replacing it with jackson should ensure that the function is consistent with the Release version, so the configuration item READ_UNKNOWN_ENUM_VALUES_AS_NULL is opened

经过测试 1.3.0 Release 版本没有这个问题,dev 有这个问题。
是因为 1.3.0 Release 用的 fastjson,fastjson对 enum 反序列化做了适配。
现在替换为 jackson 应该保证功能和Release版本一致,所以打开了这个配置项READ_UNKNOWN_ENUM_VALUES_AS_NULL

Hi, thx a lot for your work.
I've checked this error, it is fastjson adapted to the deserialization of enum.
In my opinion,

  1. I think it's not appropriate to adapted when the read the unknown enum value and return null because it will swallow all the enum deserialization error by using JSONUtils and cause some unforeseen errors;
  2. And in general, this kind of data error due to the front-end transmission should be fail fast, and not suitable for subsequent adaptation;
    Can you help to fix this in the transmission part of the front-end ?

我也检查了一遍,确实是fastjson在反序列化时做了适配。
个人理解,

  1. 这样的适配的应该是没有必要的,因为这样的适配会导致所有用到JSONUtils的枚举值序列化错误都会被吞掉,可能会导致一些不可预见的错误;
  2. 一般情况下这类由于前端传输的数据错误应该都是fail-fast的,而不适合后续去做适配;
    你可以从前端传输数据的角度去修复这个问题,把这种情况避免吗?

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jul 2, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@davidzollo
Copy link
Copy Markdown
Contributor

as a global project, English is our official language

@davidzollo
Copy link
Copy Markdown
Contributor

I think this fix PR will make the program more robust, and it's not big problem

@apache apache deleted a comment from muzhongjiang Jul 2, 2020
@davidzollo
Copy link
Copy Markdown
Contributor

davidzollo commented Jul 2, 2020

By the way , fastjson will be removed from DolphinScheduler

@davidzollo davidzollo merged commit a9eb7b1 into apache:dev Jul 2, 2020
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

Successfully merging this pull request may close these issues.

3 participants