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

terminal status is not restored when dlv "die" #73

Closed
raff opened this issue Mar 2, 2015 · 4 comments
Closed

terminal status is not restored when dlv "die" #73

raff opened this issue Mar 2, 2015 · 4 comments
Labels
Milestone

Comments

@raff
Copy link
Contributor

raff commented Mar 2, 2015

With the new "liner" package it is necessary to call liner.Close() to restore the terminal (enable echo, etc.)

While the code does call "defer line.Close()", the line.Close() is not executed when the application terminates because of an error (all the calls to "die() terminate with os.Exit() that doesn't execute any deferred code)

I suggest to either add some "context" do the "die" method (pass the liner or pass some context object if there are other components that need to be released) or use some package that implements "atexit" functionalities.

If you have a preferred method I can work on a pull request.

@derekparker derekparker added this to the 1.0 milestone Mar 2, 2015
@derekparker
Copy link
Member

Yeah I'm fine with passing along line to die, I think it makes sense to put the responsibility in that function to restore terminal state, since it's the one calling os.Exit.

@raff
Copy link
Contributor Author

raff commented Mar 2, 2015

ok, I'll prepare a pull request and submit it (actually I already did this
some time ago... and then managed to delete all my changes ):

On Mon, Mar 2, 2015 at 10:13 AM, Derek Parker notifications@github.com
wrote:

Yeah I'm fine with passing along line to die, I think it makes sense to
put the responsibility in that function to restore terminal state, since
it's the one calling os.Exit.


Reply to this email directly or view it on GitHub
https://github.com/derekparker/delve/issues/73#issuecomment-76766814.

@pnasrat
Copy link
Contributor

pnasrat commented Mar 2, 2015

Thanks I noticed this in my testing also.

@derekparker
Copy link
Member

This is fixed.

nclifton pushed a commit to nclifton/delve that referenced this issue Feb 24, 2021
* initial motar version of webhook service implementation

* added webhook mms test

* implemented mortar webhook tests for optout

* spelling correction

* removed tests

* isolate webhook

* fix webhook config

* fix webhook port

* ports

* new docker dev

* adjustments to webhook service env values

* remove mortar

* envs

* passing context to db for webhook

* fix docker.dev

* renamed/moved `webhook/rpc/grpc` to `webhook/rpc/webhookpb`

* merged crud transforms into webhook.go

* reorganise rpc dependencies

* vendor updates

* cleanup mortar

* update webhook readme and grpc code generate script

* updated webhook generate  script and readme

* - update to webhook readme
 - fix to mm7 integration test

* modd working

* tidy

* reorganise and rename webhook db file

* reorganise const and add service file

* Merge branch 'origin/176048615-webhook-service-mortar-grpc'

* update gitignore

* use hostname

* prod staging conf

* remove client aliases

* 176180991 added unit test for webhook (go-delve#69)

* added unit test using faker for fake data generation

* removed faker

* adjust error output

* removed TODO comment and blank line

* 176180995-Webhook-testing-integration-for-CRUD-endpoints (go-delve#73)

* added unit test using faker for fake data generation

* initial

* Merge branch '176180991-grpc-webhook-unit-testing'

* Merge remote-tracking branch 'origin/176048615-webhook-service-mortar-grpc'

* Merge remote-tracking branch 'origin/176048615-webhook-service-mortar-grpc'

* further work - still not running to completion

* basically runs now, some mysteries left and deeper testing required

* attempts to understand issues with rabbit connection closing

* integration test running and closing containers

* refactored integration test fixtures out into a sub-package

* added integration test readme

* added database assertions assertdb package

* implemented CRUD integration test for Webhook service Find

* some test fixture support utils refactoring

* added update and delete tets to webhook service integration test

* Added a github workflow

* Fix up linting

* - work on using env variables for integration test runs
 - improved test teardowns to clean up database tables

* Update backend.yml

adding environment variables to workflow file

* Update backend.yml

adjusted env var WEBHOOK_MIGRATION_ROOT

* Update README.md

seeing what a badge looks like for the backend workflow

* Update README.md

backend workflow badge

* Squashed commit of the following:

commit d561def3bda45039f5c2e97c84ced6b2b8da6d98
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 08:52:24 2020 +1100

    updaded readme

* readme updates

* more readme updates

* readme update

* readme update

* major surgery on webhook service to allow purer integration testing of the service startup

* removed todo

* put the integration build tag back in for the github workflow to select the integration tests to run in the special environment for them

* same as before - adding integratio build tag back

* using database/sql

* picked off some lint

* different message from Update when the account id or id is not found

* adjusted update webhook db func to retrurn "not found" when no rowss are in the results set

Co-authored-by: Mike Bissett <mike@burstsms.com>

* merged integration testing

* go mod vendor

* 176181251-Webhook-testing-integration-for-publish-endpoints (go-delve#76)

* added unit test using faker for fake data generation

* initial

* Merge branch '176180991-grpc-webhook-unit-testing'

* Merge remote-tracking branch 'origin/176048615-webhook-service-mortar-grpc'

* Merge remote-tracking branch 'origin/176048615-webhook-service-mortar-grpc'

* further work - still not running to completion

* basically runs now, some mysteries left and deeper testing required

* attempts to understand issues with rabbit connection closing

* integration test running and closing containers

* refactored integration test fixtures out into a sub-package

* added integration test readme

* added database assertions assertdb package

* implemented CRUD integration test for Webhook service Find

* some test fixture support utils refactoring

* added update and delete tets to webhook service integration test

* Added a github workflow

* Fix up linting

* - work on using env variables for integration test runs
 - improved test teardowns to clean up database tables

* Update backend.yml

adding environment variables to workflow file

* Update backend.yml

adjusted env var WEBHOOK_MIGRATION_ROOT

* Update README.md

seeing what a badge looks like for the backend workflow

* Update README.md

backend workflow badge

* updaded readme

* Squashed commit of the following:

commit d561def3bda45039f5c2e97c84ced6b2b8da6d98
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 08:52:24 2020 +1100

    updaded readme

* readme updates

* more readme updates

* readme update

* readme update

* Merge branch '176180995-Webhook-testing-integration-for-CRUD-endpoints'

* Squashed commit of the following:

commit b0313d93dfb05c77b8bd9dc8c9473cb6c77ac0dd
Author: neil <neil@burstsms.com>
Date:   Wed Dec 23 16:08:51 2020 +1100

    adjusted update webhook db func to retrurn "not found" when no rowss are in the results set

commit e15b9b35dea9804ad8c7aa8ea471246e33e4cb6f
Author: neil <neil@burstsms.com>
Date:   Wed Dec 23 15:43:51 2020 +1100

    different message from Update when the account id or id is not found

commit f2b9c2ca70e3905e21fe5e2254f486675dde95bf
Author: neil <neil@burstsms.com>
Date:   Wed Dec 23 15:34:25 2020 +1100

    picked off some lint

commit ce3fca3c5c0480804f6a9d32ae0869e9b3cbed01
Author: neil <neil@burstsms.com>
Date:   Wed Dec 23 15:26:29 2020 +1100

    using database/sql

commit 23c71aa371e3c11f091804544e01871fb781cfbf
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 19:58:29 2020 +1100

    same as before - adding integratio build tag back

commit 257f0290d32f069c1320e204015fe88706bf4426
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 19:54:24 2020 +1100

    put the integration build tag back in for the github workflow to select the integration tests to run in the special environment for them

commit 16857df1cb701a0e6c7c14924c6cbd6298ff6c59
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 19:45:21 2020 +1100

    removed todo

commit 9bd7910bf12c77d19d13e5234760a8fa81322180
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 19:38:04 2020 +1100

    major surgery on webhook service to allow purer integration testing of the service startup

* added worker and httpServer to check webhook requests

* lint problem fixed

* another lint fix

* assertbody json of request sent to webhook url

* updated expected webhook http request. NOTE: this does not currently match the spec

Co-authored-by: Mike Bissett <mike@burstsms.com>

* restored old main.go files - is boken now

* fixed webhook server main.go

* oops restored wrong main.go

* fixed unit tests and tecloo api pasreSubmit ContentID parse

* re-instated providing a different source message based on message type sms/mms

* OptOutViaMsg return err when cannot provide the origin/source Message

* removed output of soap variable from test

Co-authored-by: neil <neil@burstsms.com>
Co-authored-by: pjserol <pjserol@gmail.com>
Co-authored-by: Mike Bissett <mike@burstsms.com>
nclifton pushed a commit to nclifton/delve that referenced this issue Feb 24, 2021
* initial motar version of webhook service implementation

* added webhook mms test

* implemented mortar webhook tests for optout

* spelling correction

* removed tests

* isolate webhook

* fix webhook config

* fix webhook port

* ports

* new docker dev

* adjustments to webhook service env values

* remove mortar

* envs

* passing context to db for webhook

* fix docker.dev

* renamed/moved `webhook/rpc/grpc` to `webhook/rpc/webhookpb`

* merged crud transforms into webhook.go

* reorganise rpc dependencies

* vendor updates

* cleanup mortar

* update webhook readme and grpc code generate script

* updated webhook generate  script and readme

* - update to webhook readme
 - fix to mm7 integration test

* modd working

* tidy

* reorganise and rename webhook db file

* reorganise const and add service file

* Merge branch 'origin/176048615-webhook-service-mortar-grpc'

* update gitignore

* use hostname

* prod staging conf

* remove client aliases

* add fake webhook endpoint for tracing

* 176180991 added unit test for webhook (go-delve#69)

* added unit test using faker for fake data generation

* removed faker

* adjust error output

* removed TODO comment and blank line

* add tracing middleware rpc client and server

* rest api tracing, fix jaeger setup

* rabbit tracing

* attempt to extend opentracing nethttp

* move gzip and json header functionality to tracer middleware

* log rabbit msg

* add gRPC logs request and response with otgrpc for tracing

* log grpc req/res

* rabbit consumer trace fix

* fix format http header response

* 176180995-Webhook-testing-integration-for-CRUD-endpoints (go-delve#73)

* added unit test using faker for fake data generation

* initial

* Merge branch '176180991-grpc-webhook-unit-testing'

* Merge remote-tracking branch 'origin/176048615-webhook-service-mortar-grpc'

* Merge remote-tracking branch 'origin/176048615-webhook-service-mortar-grpc'

* further work - still not running to completion

* basically runs now, some mysteries left and deeper testing required

* attempts to understand issues with rabbit connection closing

* integration test running and closing containers

* refactored integration test fixtures out into a sub-package

* added integration test readme

* added database assertions assertdb package

* implemented CRUD integration test for Webhook service Find

* some test fixture support utils refactoring

* added update and delete tets to webhook service integration test

* Added a github workflow

* Fix up linting

* - work on using env variables for integration test runs
 - improved test teardowns to clean up database tables

* Update backend.yml

adding environment variables to workflow file

* Update backend.yml

adjusted env var WEBHOOK_MIGRATION_ROOT

* Update README.md

seeing what a badge looks like for the backend workflow

* Update README.md

backend workflow badge

* Squashed commit of the following:

commit d561def3bda45039f5c2e97c84ced6b2b8da6d98
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 08:52:24 2020 +1100

    updaded readme

* readme updates

* more readme updates

* readme update

* readme update

* major surgery on webhook service to allow purer integration testing of the service startup

* removed todo

* put the integration build tag back in for the github workflow to select the integration tests to run in the special environment for them

* same as before - adding integratio build tag back

* using database/sql

* picked off some lint

* different message from Update when the account id or id is not found

* adjusted update webhook db func to retrurn "not found" when no rowss are in the results set

Co-authored-by: Mike Bissett <mike@burstsms.com>

* merged integration testing

* go mod vendor

* 176181251-Webhook-testing-integration-for-publish-endpoints (go-delve#76)

* added unit test using faker for fake data generation

* initial

* Merge branch '176180991-grpc-webhook-unit-testing'

* Merge remote-tracking branch 'origin/176048615-webhook-service-mortar-grpc'

* Merge remote-tracking branch 'origin/176048615-webhook-service-mortar-grpc'

* further work - still not running to completion

* basically runs now, some mysteries left and deeper testing required

* attempts to understand issues with rabbit connection closing

* integration test running and closing containers

* refactored integration test fixtures out into a sub-package

* added integration test readme

* added database assertions assertdb package

* implemented CRUD integration test for Webhook service Find

* some test fixture support utils refactoring

* added update and delete tets to webhook service integration test

* Added a github workflow

* Fix up linting

* - work on using env variables for integration test runs
 - improved test teardowns to clean up database tables

* Update backend.yml

adding environment variables to workflow file

* Update backend.yml

adjusted env var WEBHOOK_MIGRATION_ROOT

* Update README.md

seeing what a badge looks like for the backend workflow

* Update README.md

backend workflow badge

* updaded readme

* Squashed commit of the following:

commit d561def3bda45039f5c2e97c84ced6b2b8da6d98
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 08:52:24 2020 +1100

    updaded readme

* readme updates

* more readme updates

* readme update

* readme update

* Merge branch '176180995-Webhook-testing-integration-for-CRUD-endpoints'

* Squashed commit of the following:

commit b0313d93dfb05c77b8bd9dc8c9473cb6c77ac0dd
Author: neil <neil@burstsms.com>
Date:   Wed Dec 23 16:08:51 2020 +1100

    adjusted update webhook db func to retrurn "not found" when no rowss are in the results set

commit e15b9b35dea9804ad8c7aa8ea471246e33e4cb6f
Author: neil <neil@burstsms.com>
Date:   Wed Dec 23 15:43:51 2020 +1100

    different message from Update when the account id or id is not found

commit f2b9c2ca70e3905e21fe5e2254f486675dde95bf
Author: neil <neil@burstsms.com>
Date:   Wed Dec 23 15:34:25 2020 +1100

    picked off some lint

commit ce3fca3c5c0480804f6a9d32ae0869e9b3cbed01
Author: neil <neil@burstsms.com>
Date:   Wed Dec 23 15:26:29 2020 +1100

    using database/sql

commit 23c71aa371e3c11f091804544e01871fb781cfbf
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 19:58:29 2020 +1100

    same as before - adding integratio build tag back

commit 257f0290d32f069c1320e204015fe88706bf4426
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 19:54:24 2020 +1100

    put the integration build tag back in for the github workflow to select the integration tests to run in the special environment for them

commit 16857df1cb701a0e6c7c14924c6cbd6298ff6c59
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 19:45:21 2020 +1100

    removed todo

commit 9bd7910bf12c77d19d13e5234760a8fa81322180
Author: neil <neil@burstsms.com>
Date:   Tue Dec 22 19:38:04 2020 +1100

    major surgery on webhook service to allow purer integration testing of the service startup

* added worker and httpServer to check webhook requests

* lint problem fixed

* another lint fix

* assertbody json of request sent to webhook url

* updated expected webhook http request. NOTE: this does not currently match the spec

Co-authored-by: Mike Bissett <mike@burstsms.com>

* restored old main.go files - is boken now

* fixed webhook server main.go

* revert http resp logging changes

* add fake webhook endpoint for tracing

* add tracing middleware rpc client and server

* rest api tracing, fix jaeger setup

* rabbit tracing

* attempt to extend opentracing nethttp

* move gzip and json header functionality to tracer middleware

* log rabbit msg

* add gRPC logs request and response with otgrpc for tracing

* log grpc req/res

* rabbit consumer trace fix

* fix format http header response

* revert http resp logging changes

* lint

* worker tracer fix

Co-authored-by: neil <neil@burstsms.com>
Co-authored-by: pjserol <pjserol@gmail.com>
Co-authored-by: Mike Bissett <mike@burstsms.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants