You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "lidavidm (via GitHub)" <gi...@apache.org> on 2023/06/06 14:31:08 UTC
[GitHub] [arrow-adbc] lidavidm opened a new pull request, #730: feat(go/adbc/pkg): catch panics at interface boundary
lidavidm opened a new pull request, #730:
URL: https://github.com/apache/arrow-adbc/pull/730
Fixes #718.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on pull request #730: feat(go/adbc/pkg): catch panics at interface boundary
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #730:
URL: https://github.com/apache/arrow-adbc/pull/730#issuecomment-1578900549
Hmm, atomic.Bool is go 1.19
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on pull request #730: feat(go/adbc/pkg): catch panics at interface boundary
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #730:
URL: https://github.com/apache/arrow-adbc/pull/730#issuecomment-1578881418
cc @zeroshade for first opinions
For testing, I plan to create a dummy Go driver that just panics, and then exercise it from Python (though without a way to 'reset' the driver we'll have to see what we can do there)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #730: feat(go/adbc/pkg): catch panics at interface boundary
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #730:
URL: https://github.com/apache/arrow-adbc/pull/730#discussion_r1219938751
##########
go/adbc/pkg/_tmpl/driver.go.tmpl:
##########
@@ -272,10 +342,15 @@ func {{.Prefix}}ConnectionInit(cnxn *C.struct_AdbcConnection, db *C.struct_AdbcD
}
//export {{.Prefix}}ConnectionRelease
-func {{.Prefix}}ConnectionRelease(cnxn *C.struct_AdbcConnection, err *C.struct_AdbcError) C.AdbcStatusCode {
+func {{.Prefix}}ConnectionRelease(cnxn *C.struct_AdbcConnection, err *C.struct_AdbcError) (code C.AdbcStatusCode) {
if !checkConnAlloc(cnxn, err, "AdbcConnectionRelease") {
return C.ADBC_STATUS_INVALID_STATE
}
+ defer func() {
+ if e := recover(); e != nil {
+ code = poison(err, "AdbcConnectionRelease", e)
+ }
+ }()
Review Comment:
and so on, not gonna comment this on every one
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #730: feat(go/adbc/pkg): catch panics at interface boundary
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #730:
URL: https://github.com/apache/arrow-adbc/pull/730#discussion_r1219937953
##########
go/adbc/pkg/_tmpl/driver.go.tmpl:
##########
@@ -237,23 +297,33 @@ func {{.Prefix}}ConnectionNew(cnxn *C.struct_AdbcConnection, err *C.struct_AdbcE
}
//export {{.Prefix}}ConnectionSetOption
-func {{.Prefix}}ConnectionSetOption(cnxn *C.struct_AdbcConnection, key, val *C.cchar_t, err *C.struct_AdbcError) C.AdbcStatusCode {
+func {{.Prefix}}ConnectionSetOption(cnxn *C.struct_AdbcConnection, key, val *C.cchar_t, err *C.struct_AdbcError) (code C.AdbcStatusCode) {
if !checkConnAlloc(cnxn, err, "AdbcConnectionSetOption") {
return C.ADBC_STATUS_INVALID_STATE
}
+ defer func() {
+ if e := recover(); e != nil {
+ code = poison(err, "AdbcConnectionSetOption", e)
+ }
+ }()
Review Comment:
should likely be before the `if !checkConnAlloc` in case anything somehow panics in there.
##########
go/adbc/pkg/_tmpl/driver.go.tmpl:
##########
@@ -237,23 +297,33 @@ func {{.Prefix}}ConnectionNew(cnxn *C.struct_AdbcConnection, err *C.struct_AdbcE
}
//export {{.Prefix}}ConnectionSetOption
-func {{.Prefix}}ConnectionSetOption(cnxn *C.struct_AdbcConnection, key, val *C.cchar_t, err *C.struct_AdbcError) C.AdbcStatusCode {
+func {{.Prefix}}ConnectionSetOption(cnxn *C.struct_AdbcConnection, key, val *C.cchar_t, err *C.struct_AdbcError) (code C.AdbcStatusCode) {
if !checkConnAlloc(cnxn, err, "AdbcConnectionSetOption") {
return C.ADBC_STATUS_INVALID_STATE
}
+ defer func() {
+ if e := recover(); e != nil {
+ code = poison(err, "AdbcConnectionSetOption", e)
+ }
+ }()
conn := getFromHandle[cConn](cnxn.private_data)
- code := errToAdbcErr(err, conn.cnxn.(adbc.PostInitOptions).SetOption(C.GoString(key), C.GoString(val)))
- return C.AdbcStatusCode(code)
+ rawCode := errToAdbcErr(err, conn.cnxn.(adbc.PostInitOptions).SetOption(C.GoString(key), C.GoString(val)))
+ return C.AdbcStatusCode(rawCode)
}
//export {{.Prefix}}ConnectionInit
-func {{.Prefix}}ConnectionInit(cnxn *C.struct_AdbcConnection, db *C.struct_AdbcDatabase, err *C.struct_AdbcError) C.AdbcStatusCode {
+func {{.Prefix}}ConnectionInit(cnxn *C.struct_AdbcConnection, db *C.struct_AdbcDatabase, err *C.struct_AdbcError) (code C.AdbcStatusCode) {
if !checkConnAlloc(cnxn, err, "AdbcConnectionInit") {
return C.ADBC_STATUS_INVALID_STATE
}
-
+ defer func() {
+ if e := recover(); e != nil {
+ code = poison(err, "AdbcConnectionInit", e)
+ }
+ }()
Review Comment:
same
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on pull request #730: feat(go/adbc/pkg): catch panics at interface boundary
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #730:
URL: https://github.com/apache/arrow-adbc/pull/730#issuecomment-1579094359
I'll fix that up, I'll leave the Go version bound as-is for now, and I'll figure out how to test this. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm merged pull request #730: feat(go/adbc/pkg): catch panics at interface boundary
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #730:
URL: https://github.com/apache/arrow-adbc/pull/730
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] zeroshade commented on pull request #730: feat(go/adbc/pkg): catch panics at interface boundary
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #730:
URL: https://github.com/apache/arrow-adbc/pull/730#issuecomment-1579055028
@lidavidm Personally I'm fine with ADBC being go1.19+ (N-1 versions of Go would be go1.19 + go1.20) if you want to use the `atomic.Bool`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org