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

Add null check for _jobServerQueue #3711

Merged
merged 4 commits into from
Jan 26, 2022

Conversation

max-zaytsev
Copy link
Contributor

@max-zaytsev max-zaytsev commented Jan 21, 2022

Added null check for the job server queue in the UpadteMetadata method.

In rare cases, the agent receives a Metadata update message from the server when the job completes and the _jobServerQueue has already been set to null. So the user can see the null refference exception in the pipeline logs:

Снимок экрана 2022-01-21 в 19 45 37

In the worker logs we can see the following:

[2022-01-19 00:54:54Z INFO JobRunner] Shutting down the job server queue.
...
[2022-01-19 00:55:08Z INFO JobServerQueue] All queue process tasks have been stopped, and all queues are drained.
...
[2022-01-19 00:55:08Z INFO JobRunner] Raising job completed event.
[2022-01-19 00:55:08Z INFO Worker] Metadata update message received.
[2022-01-19 00:55:08Z ERR  Program] System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.VisualStudio.Services.Agent.Worker.JobRunner.UpdateMetadata(JobMetadataMessage message)
   at Microsoft.VisualStudio.Services.Agent.Worker.Worker.RunAsync(String pipeIn, String pipeOut)
   at Microsoft.VisualStudio.Services.Agent.Worker.Program.MainAsync(IHostContext context, String[] args)

This PR should fix this issue


Also added 2 L0 tests:

  • DontUpdateWebConsoleLineRateIfJobServerQueueIsNull
  • UpdateWebConsoleLineRateIfJobServerQueueIsNotNull

Test results:

.\dev.cmd l0

Passed!  - Failed:     0, Passed:   765, Skipped:     0, Total:   765, Duration: 13 s - Test.dll (netcoreapp3.1)

@anatolybolshakov
Copy link
Contributor

@max-zaytsev could you please cover it by L0 tests also?

@anatolybolshakov
Copy link
Contributor

@max-zaytsev please share manual test results as well

@max-zaytsev max-zaytsev merged commit b3186b0 into master Jan 26, 2022
@max-zaytsev max-zaytsev deleted the users/max-zaytsev/null-check-update-metadata branch February 18, 2022 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants