feat(FIR-33629): Parsing structured error from response body #108
Conversation
ptiurin
changed the title
| _, err = db.Query("SET enable_json_error_output_format=true") | ||
| raiseIfError(t, err) | ||
| _, err = db.Query("SELECT 'blue'::int") | ||
| if err == nil { |
There was a problem hiding this comment.
Also verify that err is a StructuredError, otherwise it seems like plain error here whould also pass
There was a problem hiding this comment.
Unfortunately in go our error class becomes masked due to the ConstructNestedError which converts the error to string before throwing it up the stack.
However, looking through the code other error paths prepend their own prefixes. e.g. request returned non ok status code if the error code is not 200 and request returned an error in some other cases. Essentially in this test by checking the exact prefix is error during query execution: error during query request: Cannot parse string we verify the error path here by the process of elimination.
I don't think we can do better here, unless we prepend a specific prefix similar to the above. Not sure if we should do it as our errors are already pretty verbose.
There was a problem hiding this comment.
As we discussed, I believe we can be sure that if engine returns a json string and we don't parse it, the test will fail. This test is only false positive in case the backend still returns a plaintext despite of the setting, which I believe we might not account for in our testing. We can probably leave it as is
There was a problem hiding this comment.
This test is only false positive in case the backend still returns a plaintext despite of the setting
I don't think it will, since the StructuredError will not be raised so the error will be raised elsewhere in the code e.g. here we prepend the request returned non ok status code and in the test we specifically check for a prefix that doesn't contain this string. Therefore I believe the plain error will make this test fail.
In fact, I've removed the set parameters, run it and got an error:
--- FAIL: TestIncorrectQueryThrowingStructuredError (0.11s)
/Users/petrotiurin/Development/firebolt-go-sdk/driver_integration_test.go:320: Query didn't return an error with correct message, got: error during query execution: error during query request: request returned non ok status code: 400, Cannot parse string 'blue' as integer: syntax error at beginning of string
FAIL
|
Integration test needs multiple consecutive SET param fix, since otherwise a set parameter is not taken into account when verifying the second statement and it fails. However, I've tested if with a local fix (will be a separate PR) and the test verified the error correctly.