Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Return instead of panic in code that is only used in dev #2533

Merged
merged 1 commit into from
Oct 21, 2021

Conversation

catsby
Copy link
Contributor

@catsby catsby commented Oct 20, 2021

This bit of code causes an issue in at least AWS ECS server installs using on-demand runners, where a release will fail:

! Unrecognized remote plugin message:
  This usually means that the plugin is either invalid or simply
  needs to be recompiled to support the latest protocol.

The underlying issue is a panic during the init phase caused by something that's only used in development, so here we change that to simply return.

UPDATE: refactored/rebased to completely exclude internal/assets/dev.go from builds with the assetsembedded tag (see the Makefile for reference). The dev.go file was generating dev_assets.go which was not included in builds, however the dev.go file itself still was, and it's init() method could cause troubles

@catsby catsby requested a review from evanphx October 20, 2021 19:45
@github-actions github-actions bot added the core label Oct 20, 2021
@krantzinator
Copy link
Contributor

Even if for development purposes, should we still return a more friendly error message? For our future selves 😅

@catsby
Copy link
Contributor Author

catsby commented Oct 20, 2021

Even if for development purposes, should we still return a more friendly error message? For our future selves 😅

@krantzinator Normally I would say yes, however in this situation we don't have any logging or error returns at all

@mitchellh
Copy link
Contributor

This is ancient. I'd be curious if we even need this at all anymore (the dev stuff). This was basically for us to build Waypoint with go build before we had the CEB done. Since we always make install or equivalent now I wonder if we can just throw it away.

Return instead of panic in code that is only used in dev
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants