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

Fix startup error not showing up in kusto #753

Merged
merged 2 commits into from
Jun 26, 2024
Merged

Fix startup error not showing up in kusto #753

merged 2 commits into from
Jun 26, 2024

Conversation

ejizba
Copy link
Contributor

@ejizba ejizba commented Jun 24, 2024

Found this bug a couple weeks ago when Thiago was debugging a bad node v4 app on flex.

Repro steps

  1. Create a v4 app
  2. Use Node 20
  3. Delete the http trigger code and add throw new Error('test'); instead
  4. Run your app
  5. Wait a while then check app insights & kusto

Expected

The error shows up in customer's app insights and our kusto

Actual

The error only shows up in customer's app insights, but not kusto

More info

Today, we save app startup errors that happen during workerInitRequest and return them during functionsMetadataRequest or functionLoadRequest instead (which also logs them - which is what I care about for this PR). However, if the error happens for a v4 app before the v4 model is registered in user code, we skip sending the error during functionsMetadataRequest because we think it's a v3 app and the host never sends a functionLoadRequest which only applies to v3 apps.

My fix is to always log the error right away and set a flag to prevent it from potentially being duplicated later.

@ejizba ejizba requested a review from castrodd June 24, 2024 23:17
src/errors.ts Outdated Show resolved Hide resolved
@ejizba ejizba merged commit 4466637 into v3.x Jun 26, 2024
14 checks passed
@ejizba ejizba deleted the ej/startupErr branch June 26, 2024 22:20
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.

2 participants